Merge changes from BRL-CAD Main Repo #18

…and memory, but also not proceeding if we can't read next successfully.

…s potentially needed by some ifdef 0 code, which was subsequently removed - unused.

Sean spotted this, and it looks like it may have been a significant systematic mistake in my early NURBS brep coding. ON_SimpleArray can't always be used - per opennurbs_array.h, ON_SimpleArray "should not be used for arrays of classes that require explicit construction, destruction, or copy operators." That probably means we've got a lot of inappropriate uses of ON_SimpleArray at play. I tended to try and treat pointers to ON_* objects as being inserted into and managed by ON_SimpleArray essentially as if they were C arrays of pointers, and that looks to be incorrect. Good chance this is the root cause behind a lot of the more obscure memory issues clang static analyzer is finding with the various brep related codes...

Previous problem wasn't quite what I thought - assignment was indeed making a full copy, so we also need to delete. Also need a few more SimpleArray -> ClassArray switches.

…t seem to be in a working state and hasn't been worked on much since 2009.

…efore we do this bit.

…y we can clear by a combination of trimming and small changes.

…lse and then later assumes it is true for no apparent reason... just add a null check.

…copy.

…g is working well enough, so not messing with this, but commenting it out rather than deleting it in case this does become an issue in the future.

Having gotten these clean (at least with clang 12), lets keep them that way. utahrle doesn't have an active upstream, our poly2tri is significantly altered from the upstream version, and osmesa is an extraction of Mesa 7.0.4's OpenGL 2.1 software-only rasterizer - the current upstream code is now completely different. Realistically Tktable and Tkhtml (particularly Tkhtml) should also fall into this category, but they're not as trivial to set up in the test script since they may need Tcl/Tk built from src/other/ext first (and, unlike the other three libs, the Tk widgets should eventually be phased out in favor of Qt functionality anyway...) OpenNURBS is in a separate class - basically, unlike the other src/other libs not in ext, it does have an active upstream - we just need to maintain it in src/other because of the significant number of patches we apply to it.

Grr. Getting random failures in the stepcode building, mostly on the Github runners but occasionally on normal desktops as well. Symptoms are an internal operation of the stepcode build erring out complaining about not being able to do I/O on one of the files that needs to be copied, but when you check the file is there. Historically that's happened when disk I/O isn't quite keeping up with processing or parallel operations end up colliding. Traditional solution has been to add sentinel files that are only created after the original operation is done. See if that helps here.

Including librt headers pulls in openNURBS, which has a lot of implications for things like needing warning suppressed. Use an ifdef guard to only expose and use these functions where necessary to limit the scope of the necessary adjustment.

With the count down, it's no longer as beneficial to separate things out into per-library reports - just assemble a single report for the all target, and use the total report count as a criteria for failure - like our other checks, if the count rises over the current known rate report failure. We'll lower the number as issues are resolved.

… when trying to satisfy the static analyzer - revert until the correct change is set up.

…on size for the two previously using int.

libtclstub ends up with an unresolved symbol as TclIsSpaceProc is not available in the stub lib. this causes mac build to fail hard on anything linking the stub lib (like tk).

…for the flag test to succeed. Leaving commented out as actually enabling the flag appears to cause breakage.

Well this is wacky... apparently adding the -fsanitize=address flag changes the results of the FindThreads test, causing it to not add -lpthread. If we don't use the flag consistently everywhere after we start using that flag the address+Threads result will break independent logic that's not using the -fsanatize setting.

…e.cpp:3149

We don't want this to accidentally activate if -lasan is no longer a requirement someday - BRL-CAD's code isn't ready for it yet. In particular, the LeakSanitizer trips up on the rt_free_rti/rt_clean tangle as well as _db_walk_subtree. The former may need elimination of the rt_uniresource global to properly clean up, and the latter is deep in the raytracing core and will have to be approached very carefully. For convenience, here are the backtraces: 889: Test command: build/bin/gqa "-P" "1" "-Ao" "-p" "ovlp_" "brlcad/src/gtools/tests/../../libged/tests/ged_tests.g" "ovlp" 889: Test timeout computed to be: 1500 889: Trying initial grid spacing: 50 mm 889: Using grid spacing lower limit: 0.005 mm 889: Plotting overlaps to ovlp_overlaps.plot3 889: Processing with grid spacing 50 mm 44 x 48 x 39 889: Summary (50mm grid spacing): 889: list Overlaps: 889: /ovlp/r1 /ovlp/r2 count:2277 dist:1780.3mm @ (-248 350 -903.325) 889: 889: ================================================================= 889: ==672620==ERROR: LeakSanitizer: detected memory leaks 889: 889: Direct leak of 524288 byte(s) in 1024 object(s) allocated from: 889: #0 0x498087 in posix_memalign (build/bin/gqa+0x498087) 889: #1 0x7f9d3ca476c3 in alloc brlcad/src/libbu/malloc.c:137:10 889: #2 0x7f9d3ca478f6 in bu_calloc brlcad/src/libbu/malloc.c:174:12 889: #3 0x7f9d3ca9b25e in bu_ptbl_init brlcad/src/libbu/ptbl.c:45:26 889: #4 0x7f9d40d38be4 in rt_init_resource brlcad/src/librt/prep.c:855:2 889: #5 0x7f9d40d3ba65 in rt_clean_resource brlcad/src/librt/prep.c:1063:5 889: #6 0x7f9d40d3239a in rt_clean brlcad/src/librt/prep.c:1210:6 889: #7 0x7f9d40d30f80 in rt_free_rti brlcad/src/librt/prep.c:170:5 889: #8 0x7f9d380c4573 (<unknown module>) 889: #9 0x7f9d424f2526 in ged_exec brlcad/src/libged/exec.cpp:83:16 889: #10 0x7f9d424fd51b in ged_gqa brlcad/src/libged/exec_mapping.cpp:177:1 889: #11 0x4c8b17 in main brlcad/src/gtools/gqa.c:105:11 889: #12 0x7f9d3c6be0b2 in __libc_start_main /build/glibc-eX1tMB/glibc-2.31/csu/../csu/libc-start.c:308:16 889: 889: Direct leak of 524288 byte(s) in 1024 object(s) allocated from: 889: #0 0x498087 in posix_memalign (build/bin/gqa+0x498087) 889: #1 0x7f9d3ca476c3 in alloc brlcad/src/libbu/malloc.c:137:10 889: #2 0x7f9d3ca478f6 in bu_calloc brlcad/src/libbu/malloc.c:174:12 889: #3 0x7f9d3ca9b25e in bu_ptbl_init brlcad/src/libbu/ptbl.c:45:26 889: #4 0x7f9d40d38c65 in rt_init_resource brlcad/src/librt/prep.c:858:2 889: #5 0x7f9d380c0db2 (<unknown module>) 889: #6 0x7f9d424f2526 in ged_exec brlcad/src/libged/exec.cpp:83:16 889: #7 0x7f9d424fd51b in ged_gqa brlcad/src/libged/exec_mapping.cpp:177:1 889: #8 0x4c8b17 in main brlcad/src/gtools/gqa.c:105:11 889: #9 0x7f9d3c6be0b2 in __libc_start_main /build/glibc-eX1tMB/glibc-2.31/csu/../csu/libc-start.c:308:16 889: 889: Direct leak of 64 byte(s) in 2 object(s) allocated from: 889: #0 0x498087 in posix_memalign (build/bin/gqa+0x498087) 889: #1 0x7f9d3ca476c3 in alloc brlcad/src/libbu/malloc.c:137:10 889: #2 0x7f9d3ca478f6 in bu_calloc brlcad/src/libbu/malloc.c:174:12 889: #3 0x7f9d40ce3610 in _db_walk_subtree brlcad/src/librt/db_tree.c:1914:6 889: #4 0x7f9d40cdd664 in _db_walk_dispatcher brlcad/src/librt/db_tree.c:1992:2 889: #5 0x7f9d3ca795da in bu_parallel brlcad/src/libbu/parallel.c:574:2 889: #6 0x7f9d40cdc2a2 in db_walk_tree brlcad/src/librt/db_tree.c:2199:5 889: #7 0x7f9d41893a86 in rt_gettrees_muves brlcad/src/librt/tree.c:779:6 889: #8 0x7f9d4189bb3f in rt_gettrees_and_attrs brlcad/src/librt/tree.c:883:12 889: #9 0x7f9d4189bcba in rt_gettree brlcad/src/librt/tree.c:896:11 889: #10 0x7f9d380c0e88 (<unknown module>) 889: #11 0x7f9d424f2526 in ged_exec brlcad/src/libged/exec.cpp:83:16 889: #12 0x7f9d424fd51b in ged_gqa brlcad/src/libged/exec_mapping.cpp:177:1 889: #13 0x4c8b17 in main brlcad/src/gtools/gqa.c:105:11 889: #14 0x7f9d3c6be0b2 in __libc_start_main /build/glibc-eX1tMB/glibc-2.31/csu/../csu/libc-start.c:308:16 889: 889: Direct leak of 64 byte(s) in 2 object(s) allocated from: 889: #0 0x498087 in posix_memalign (build/bin/gqa+0x498087) 889: #1 0x7f9d3ca476c3 in alloc brlcad/src/libbu/malloc.c:137:10 889: #2 0x7f9d3ca478f6 in bu_calloc brlcad/src/libbu/malloc.c:174:12 889: #3 0x7f9d40c2a4bc in rt_comb_import5 brlcad/src/librt/comb/comb.c:518:6 889: #4 0x7f9d40c79613 in rt_db_external5_to_internal5 brlcad/src/librt/db5_io.c:958:8 889: #5 0x7f9d40c79e9f in rt_db_get_internal5 brlcad/src/librt/db5_io.c:998:11 889: #6 0x7f9d40ce6b76 in rt_db_get_internal brlcad/src/librt/dir.c:131:9 889: #7 0x7f9d40cd31b4 in db_recurse brlcad/src/librt/db_tree.c:1016:6 889: #8 0x7f9d40ce34ee in _db_walk_subtree brlcad/src/librt/db_tree.c:1898:16 889: #9 0x7f9d40cdd664 in _db_walk_dispatcher brlcad/src/librt/db_tree.c:1992:2 889: #10 0x7f9d3ca795da in bu_parallel brlcad/src/libbu/parallel.c:574:2 889: #11 0x7f9d40cdc2a2 in db_walk_tree brlcad/src/librt/db_tree.c:2199:5 889: #12 0x7f9d41893a86 in rt_gettrees_muves brlcad/src/librt/tree.c:779:6 889: #13 0x7f9d4189bb3f in rt_gettrees_and_attrs brlcad/src/librt/tree.c:883:12 889: #14 0x7f9d4189bcba in rt_gettree brlcad/src/librt/tree.c:896:11 889: #15 0x7f9d380c0e88 (<unknown module>) 889: #16 0x7f9d424f2526 in ged_exec brlcad/src/libged/exec.cpp:83:16 889: #17 0x7f9d424fd51b in ged_gqa brlcad/src/libged/exec_mapping.cpp:177:1 889: #18 0x4c8b17 in main brlcad/src/gtools/gqa.c:105:11 889: #19 0x7f9d3c6be0b2 in __libc_start_main /build/glibc-eX1tMB/glibc-2.31/csu/../csu/libc-start.c:308:16 889: 889: SUMMARY: AddressSanitizer: 1048704 byte(s) leaked in 2052 allocation(s).

…of different types (like bot_merge merged.bot *.t0 in the FAA Generic_Twin example.)

… cleaning up gedp in various places.

… but no contents - we still need to free the argv container.

Oof. Writing down the details on this one, as I doubt I'll remember... The start was turning on both the AddressSanitizer and Qt. When doing so, most programs suddenly began reporting memory leaks. However, the report was rather cryptic, being four entries similar to this one: ==1262202==ERROR: LeakSanitizer: detected memory leaks Direct leak of 21 byte(s) in 1 object(s) allocated from: #0 0x498087 in posix_memalign (build/src/libdm/tests/dm_test+0x498087) #1 0x7f4e90e53675 in alloc brlcad/src/libbu/malloc.c:129:10 #2 0x7f4e90e53400 in bu_malloc brlcad/src/libbu/malloc.c:167:12 #3 0x7f4e90ee1c02 in bu_strdupm brlcad/src/libbu/str.c:165:17 #4 0x7f4e87dcd3e9 (<unknown module>) #5 0x7f4e87dcdea5 (<unknown module>) #6 0x7f4e9650eb89 (/lib64/ld-linux-x86-64.so.2+0x11b89) I wasn't immediately sure if the unknown module error was related to the plugin loading, and spent a lot of time trying to find a string memory issue in dm_init.cpp. After that proved fruitless, other than to confirm that the error disappeared if i removed the bu_dlclose-ing of the handles saved at initialization, I began to look for ways to decode the "unknown module" entries. That led to the following issue: google/sanitizers#89 which describes the problem ASan has with dynamic libraries. Comments indicated that it would see real issues, but won't report which dynamic library they come from (and there are no plans to fix this anytime soon... grr...). In fairness, valgrind can see the error too but also has the same reporting problem; it appears to be ubiquitous. Fortunately, we have an alternative due to the way our plugin system and test apps work - we can simply add and remove .so files to the directory and see how the error reporting changes to zero in on which file(s) are triggering the problem. Doing so quickly made apparent what should have been obvious in retrospect - two of the errors each were coming from the Qt and swrast plugins, which had been off in earlier testing. Since there was no backtrace beyond the bu_strdupm call itself, and there were two errors per file, the suspect was the bu_strdup calls initializing the "char *" names for the fb structures. The C files use static strings (which is why the non-Qt plugins didn't show the issue), but C++ doesn't tolerate the type mismatch. The original hack workaround was just to bu_strdup and create a (char *) string, but as the leak detectors correctly note this also means there's no way to clean up the allocated memory. As far as I can tell there is no reason for these strings to be editable (char *) strings - the dm container's equivalents are static. This commit removes any logic assuming if_name is dynamic, and also removes the bu_strdup hack from Qt and swrast.

…esn't work when we do that.

db_str2op expects a NULL terminated const char * array - it skips leading whitespace with a while(*str) loop, so it needs to know when to stop. Use null terminated strings as input instead of pointers to individual chars.

CID 280107, 280144. A quick trial indicates that we'll need other changes if we update lemon.c and lempar.c to the latest upstream versions, so for now just mod our copies to free the memory.

CID 481205

CID 490501

CID 500450

CID 1127262. If I'm understanding this correctly, Coverity is spotting the bounds check in the debug printing statement, interpreting this as an indication that yyruleno might exceed the array size, and (assuming that's true) then points out that if yyruleno CAN do that it would result in a problem when the array contents are accessed later on. Just do the check up front, to make sure we don't get into this logic with a problematic yyruleno, and warn if we hit it. Not sure if the debugging logic was doing this check on general principles or they had an issue at some point...

CID 1309088 Memory wasn't getting freed correctly in the error case - separate out the checks so we can do the right thing in each case.

CID 1487591

follow the same pattern used throughout the file. manually nul-terminating after copying into the buffer.

putting in the appropriate WMOP really makes it evident that the ops may be wrong too based on the nearby logic and comments. leaving a FIXME note instead of updating, though as not set up to test changing it.

…d dm_put.

…splay manager.

CID 1487593 - without a way to clean up the dlopen handles, we're leaking resources. Error cases are simple - just close before returning - but if we want to persist the successful handles until we're done converting we need to be able to stash them in the context.

Option handling was broken in gcv, and the target name was wrong for enabling the GDAL test for converting a dem file.

CID 1487599 - creating a new node and then immediately overwriting it with createNewNode was leaking memory. While we're at it, add a basic regression test to make sure the VRML conversion plugin runs.

CID 1487604

CID 1487624 - If I understand this correctly, Coverity is indicating that this resource is copied rather than pointed to, and thus we need to be freeing it locally.

CID 1487628 - prompted by this, but not 100% this is the fix...

CID 718529 - need to free pathbuf even if path == NULL.

CID 1487662 CmdMgr constructor allocates memory witn hew, but didn't have a destructor to free it.

CID 1487674 - function was returning a pointer to locally scoped arrays, which means the caller would have no guarantee that the contents would remain correct. Make the arrays static, since they are are supplying fixed definitions that will not change for the life of the program.

Since we're using C++ containers and copying for these (at least for the moment) it looks like Coverity is setting some issues with uninitialized data. Not sure if this is the answer, but it's at least a little more explicit about the intent.

CID 1488258

CID 1487686 - Coverity reporting that MarchingSquares::vertexList in SetVertex has size 4, but the loop was going to 11. At a guess the 12 limit may have come from the edge count of a cube, not a square...

CID 1495127

CID 1489008

CID 1487956 - Resource leak reported by Coverity if testing fd <= 0 and fd == 0.

CID 1488379

CID 1488673 Resource leak when fd == 0

CID 1495209 - this may be a false positive in that the set of possible keys doesn't seem to include the GL_MODELVIEW_MATRX case, but go ahead and supply it anyway.

CID 1495029 If we're already at the lowest index, [i-1] doesn't work

CID 1494821, CID 1494747 - looping using VBO_ATTRIB_FIRST_MATERIAL as an index limit, which is defined as 32, not 16.

CID 1495078 - Parse_ScalarConstant's numerical path assigns the number to four slots in an array, but as the caller incremented the available output array got smaller - result, Out-of-bounds access if the numerical case activated. Rather than passing in vec directly, use a temporary array so Parse_ScalarConstant has enough room.

CID: 501626

CID: 1494906 Do here what is done for _mesa_TexEnvfi and provide a large enough array to pass to the next function.

CID 1494889

CID 1494720 CID 1494859 CID 1494865

CID 500450

CID 1494924 CID 1495118

CID 714349 - this probably indicates some other issue, since these are all constants and I wouldn't expect any invalid combinations...

CID 1495416: pathbuf may get assigned a different value during processing, so use a local variable to ensure we always free what was originally malloced.

CID 1487777 Per Coverity, free of address-of expression: moveTrisToNodes frees incorrect pointer root.

CID 1488304 and probably a few others... Use after free errors.

CID 1494693

CID 1488409

CID 1489147 - had a free call where we didn't need it. obj_array appears to function locally within each block, so use local variables to help avoid introducing this issue. I suspect we aren't cleaning up the memory fully in all cases here...

CID 1488895 - commit prompted by this issue, but not 100% sure this is the fix. Looks like multiple loops are hitting a case where arbn->eqn and new_planes contain the same pointer, but not immediately obvious what's prompting that. First guess is something stale in the new_plane pointer, which was defined at the top level function scope, so bringing it local to make behavior clearer.

CID 1488175 is indicating pc->pkc_inbuf isn't NULL terminated because it's contents come from read (which is true) and therefore snprintf has a problem with it. That would be true if we were trying to print the pkc_inbuf contents as a string, but I think the intent here is to print pointers, which should make NULL termination moot. (Printing the pointers here is of questionable utility, but that's another discussion...) See if using %p is more acceptable to Coverity.

CID 1488979 - tt_name is a 64 char array in the struct, not a dynamic string.

CID 1488030 - using the function that frees the vls contents and the vls container, but we just need the contents here.

CID 1488594 I think this might be a false positive, as CLAMP should be keeping pl->blades within bounds, but go ahead and also make sure blade never gets to BLADE_MAX to see if that helps.

CID 1487758 - Need to close fd if we got it from open()

CID 1488378

CID 1487860

CID 1489196

CID 1488507

CID 1488217 If we are representing booleans with a single char, need a more convenient way to get the corresponding db_opt_t to avoid getting into trouble with out-of-bounds access issues.

CID 1487984

CID 1488894

CID 1487801

CID 1488266 fb_rect.c:158 - overrunning array of 24576 bytes at byte offset 73725

CID 1494729

CID 1488415

CID 1488493 Coverity doesn't like doing strlen on the fread results due to possibly not being NULL terminated. I think this may be a false positive? I would expect the memset on the buffer to give us a guaranteed null at buf[BUFFER_SIZE]...

CID 1495191

CID 1494773

CID 1487921

CID 1487884

CID 1488763 If stash->connection is already closed, don't keep working with it.

CID 1488494 - if c == next_code == 4096, then Expand(c) triggers an OVERRUN condition.

…ed failing. Try increasing table size, and use bu_log to print the error code (Message is mangling the number)

Update to art with a slew of significant changes from Chris McGregor This adds support for the set command to control the number of samples, it lets are utilize OSL shaders from appleseed that are set on objects via material objects (optical "OSL" property), and it should support most of the RTUIF commands (e.g., az/el) although the code was reverted to a +y coordinate frame which results in a 90-degree rotated output image. Most significant in this pull is the conversion to registering each region individually as an appleseed object, so material properties and color can be applied per-object. There is one big performance-impacting hack in the code -- a per ray string comparison. Current librt rt_shootray() hit callback is initialized with the whole scene, but needs to report only hits for a single specific object at a time. Hack was to check if it's the right object by way of a string path-to-region comparison. Needs separate per-object rtip's or a numeric handle (e.g., pointer) to each region otherwise (depending on whether we think librt or appleseed's spatial partitioning is faster). Outstanding work from Chris.

CID 1488969 - Per Coverity: internal representation of local output escapes into output_file, but is destroyed when it exits scope. Make a C copy of the string for subsequent use, and free it when no longer needed.

CID 1494811

CID 1488659

CID 1489084 - resource leak

Commits on Nov 17, 2021

Check for null inputs

starseeker committed Nov 17, 2021

Configuration menu

View commit details

Copy full SHA for b002a79

Browse repository at this point

Copy the full SHA

b002a79 View commit details

Browse the repository at this point in the history

Commits on Nov 28, 2021

Fix distclean name.

starseeker committed Nov 28, 2021

Configuration menu

View commit details

Copy full SHA for 79ff1d3

Browse repository at this point

Copy the full SHA

79ff1d3 View commit details

Browse the repository at this point in the history

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Merge changes from BRL-CAD Main Repo #18

Merge changes from BRL-CAD Main Repo #18

Commits on Nov 17, 2021

Commits on Nov 18, 2021

Commits on Nov 19, 2021

Commits on Nov 20, 2021

Commits on Nov 22, 2021

Commits on Nov 23, 2021

Commits on Nov 28, 2021

Commits on Nov 29, 2021

Commits on Nov 30, 2021

Commits on Dec 1, 2021

Commits on Dec 7, 2021

Commits on Dec 8, 2021

Commits on Dec 9, 2021

Commits on Dec 10, 2021

Commits on Dec 11, 2021

Commits on Dec 13, 2021

Commits on Dec 14, 2021

Commits on Dec 15, 2021