Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Please MSAN #395

Closed
wants to merge 15 commits into from
Closed

Please MSAN #395

wants to merge 15 commits into from

Conversation

Algunenano
Copy link
Member 

SUMMARY: MemorySanitizer: use-of-uninitialized-value /home/raul/dev/public/postgis/liblwgeom/lwgeom_geos.c:960:6 in compare_by_envarea
==9239==WARNING: MemorySanitizer: use-of-uninitialized-value
    #0 0x55e44ad0691b in compare_by_envarea /home/raul/dev/public/postgis/liblwgeom/lwgeom_geos.c:959:6
    #1 0x7eff5a46d7c4 in msort_with_tmp.part.0 (/usr/lib/libc.so.6+0x397c4)
    #2 0x7eff5a46d895 in __GI___qsort_r (/usr/lib/libc.so.6+0x39895)
    #3 0x55e44aced5ef in findFaceHoles /home/raul/dev/public/postgis/liblwgeom/lwgeom_geos.c:971:2
    #4 0x55e44acec356 in LWGEOM_GEOS_buildArea /home/raul/dev/public/postgis/liblwgeom/lwgeom_geos.c:1125:2
    #5 0x55e44acefc7c in lwgeom_buildarea /home/raul/dev/public/postgis/liblwgeom/lwgeom_geos.c:1170:7
    #6 0x55e44a67d11b in buildarea7 /home/raul/dev/public/postgis/liblwgeom/cunit/cu_buildarea.c:314:9
    #7 0x7eff5b3fd117  (/usr/lib/libcunit.so.1+0x4117)
    #8 0x7eff5b3fd3b1  (/usr/lib/libcunit.so.1+0x43b1)
    #9 0x7eff5b3fd7b6 in CU_run_all_tests (/usr/lib/libcunit.so.1+0x47b6)
    #10 0x55e44a82fdbe in main /home/raul/dev/public/postgis/liblwgeom/cunit/cu_tester.c:177:13
    #11 0x7eff5a458222 in __libc_start_main (/usr/lib/libc.so.6+0x24222)
    #12 0x55e44a5c407d in _start (/home/raul/dev/public/postgis/liblwgeom/cunit/cu_tester+0x2607d)

  Uninitialized value was stored to memory at
    #0 0x55e44ad06863 in compare_by_envarea /home/raul/dev/public/postgis/liblwgeom/lwgeom_geos.c:957:9
    #1 0x7eff5a46d7c4 in msort_with_tmp.part.0 (/usr/lib/libc.so.6+0x397c4)

  Uninitialized value was created by a heap allocation
    #0 0x55e44a5f3c5d in __interceptor_malloc (/home/raul/dev/public/postgis/liblwgeom/cunit/cu_tester+0x55c5d)
    #1 0x55e44aab99b1 in default_allocator /home/raul/dev/public/postgis/liblwgeom/lwutil.c:91:14
    #2 0x55e44aabd1f3 in lwalloc /home/raul/dev/public/postgis/liblwgeom/lwutil.c:229:14
    #3 0x55e44acecbf9 in newFace /home/raul/dev/public/postgis/liblwgeom/lwgeom_geos.c:923:12
    #4 0x55e44acec11e in LWGEOM_GEOS_buildArea /home/raul/dev/public/postgis/liblwgeom/lwgeom_geos.c:1122:14
    #5 0x55e44acefc7c in lwgeom_buildarea /home/raul/dev/public/postgis/liblwgeom/lwgeom_geos.c:1170:7
    #6 0x55e44a67d11b in buildarea7 /home/raul/dev/public/postgis/liblwgeom/cunit/cu_buildarea.c:314:9
    #7 0x7eff5b3fd117  (/usr/lib/libcunit.so.1+0x4117)

@Komzpa
Copy link
Member

Komzpa commented Apr 17, 2019

will we msan on travis?

@Algunenano
Copy link
Member Author

It's not there yet.

Before this patch I see 120 warnings in liblwgeom unit tests and after this 67 warnings, but the remaining ones are related to either json or geos, so to investigate those I'd need to build those dependencies with MSAN (and without optimizations if possible) and check whether they are bugs and how/where to patch them.
Then, the next big thing would be building postgres also with MSAN so the regress tests can be run under it too, but that would require cleaning all the msan warnings in postresql first. Not an easy task.

@Algunenano
Please MSAN
dd5fb09 
SUMMARY: MemorySanitizer: use-of-uninitialized-value /home/raul/dev/public/postgis/liblwgeom/lwgeom_geos.c:960:6 in compare_by_envarea
==9239==WARNING: MemorySanitizer: use-of-uninitialized-value
    #0 0x55e44ad0691b in compare_by_envarea /home/raul/dev/public/postgis/liblwgeom/lwgeom_geos.c:959:6
    postgis#1 0x7eff5a46d7c4 in msort_with_tmp.part.0 (/usr/lib/libc.so.6+0x397c4)
    postgis#2 0x7eff5a46d895 in __GI___qsort_r (/usr/lib/libc.so.6+0x39895)
    postgis#3 0x55e44aced5ef in findFaceHoles /home/raul/dev/public/postgis/liblwgeom/lwgeom_geos.c:971:2
    postgis#4 0x55e44acec356 in LWGEOM_GEOS_buildArea /home/raul/dev/public/postgis/liblwgeom/lwgeom_geos.c:1125:2
    postgis#5 0x55e44acefc7c in lwgeom_buildarea /home/raul/dev/public/postgis/liblwgeom/lwgeom_geos.c:1170:7
    postgis#6 0x55e44a67d11b in buildarea7 /home/raul/dev/public/postgis/liblwgeom/cunit/cu_buildarea.c:314:9
    postgis#7 0x7eff5b3fd117  (/usr/lib/libcunit.so.1+0x4117)
    postgis#8 0x7eff5b3fd3b1  (/usr/lib/libcunit.so.1+0x43b1)
    postgis#9 0x7eff5b3fd7b6 in CU_run_all_tests (/usr/lib/libcunit.so.1+0x47b6)
    postgis#10 0x55e44a82fdbe in main /home/raul/dev/public/postgis/liblwgeom/cunit/cu_tester.c:177:13
    postgis#11 0x7eff5a458222 in __libc_start_main (/usr/lib/libc.so.6+0x24222)
    postgis#12 0x55e44a5c407d in _start (/home/raul/dev/public/postgis/liblwgeom/cunit/cu_tester+0x2607d)

  Uninitialized value was stored to memory at
    #0 0x55e44ad06863 in compare_by_envarea /home/raul/dev/public/postgis/liblwgeom/lwgeom_geos.c:957:9
    postgis#1 0x7eff5a46d7c4 in msort_with_tmp.part.0 (/usr/lib/libc.so.6+0x397c4)

  Uninitialized value was created by a heap allocation
    #0 0x55e44a5f3c5d in __interceptor_malloc (/home/raul/dev/public/postgis/liblwgeom/cunit/cu_tester+0x55c5d)
    postgis#1 0x55e44aab99b1 in default_allocator /home/raul/dev/public/postgis/liblwgeom/lwutil.c:91:14
    postgis#2 0x55e44aabd1f3 in lwalloc /home/raul/dev/public/postgis/liblwgeom/lwutil.c:229:14
    postgis#3 0x55e44acecbf9 in newFace /home/raul/dev/public/postgis/liblwgeom/lwgeom_geos.c:923:12
    postgis#4 0x55e44acec11e in LWGEOM_GEOS_buildArea /home/raul/dev/public/postgis/liblwgeom/lwgeom_geos.c:1122:14
    postgis#5 0x55e44acefc7c in lwgeom_buildarea /home/raul/dev/public/postgis/liblwgeom/lwgeom_geos.c:1170:7
    postgis#6 0x55e44a67d11b in buildarea7 /home/raul/dev/public/postgis/liblwgeom/cunit/cu_buildarea.c:314:9
    postgis#7 0x7eff5b3fd117  (/usr/lib/libcunit.so.1+0x4117)
@Algunenano
MSAN: Silence geojson warnings
5da2a5f
@Algunenano
liblwgeom: Avoid link issues in cunit
a8e3481
@Algunenano
lwout_twkb.c: Address implicit type conversion with value change
6c22396 
lwout_twkb.c:170:9: runtime error: implicit conversion from type 'long long' of value 10000000000 (64-bit, signed) to type 'int' changed the value to 1410065408 (32-bit, signed)
@Algunenano Algunenano force-pushed the msan_buildarea branch 2 times, most recently from e6eb62d to 2f2bfa3 Compare April 23, 2019 14:01
@Algunenano
lwout_wkb.c: Address implicit type conversion with value change
0d3b489 
lwout_wkb.c:288:16: runtime error: implicit conversion from type 'char' of value -16 (8-bit, signed) to type 'uint8_t' (aka 'unsigned char') changed the value to 240 (8-bit, unsigned)
lwout_wkb.c:654:27: runtime error: implicit conversion from type 'uint32_t' (aka 'unsigned int') of value 2147483663 (32-bit, unsigned) to type 'int' changed the value to -2147483633 (32-bit, signed)
lwout_wkb.c:213:16: runtime error: implicit conversion from type 'char' of value -128 (8-bit, signed) to type 'uint8_t' (aka 'unsigned char') changed the value to 128 (8-bit, unsigned)
lwout_wkb.c:607:27: runtime error: implicit conversion from type 'uint32_t' (aka 'unsigned int') of value 2147483651 (32-bit, unsigned) to type 'int' changed the value to -2147483645 (32-bit, signed)
@Algunenano
Add implicit-conversion sanitizer to the clang ubsan build
644ca54
@Algunenano
shp2pgsql-core: Address implicit type conversion with value change
ccea341 
shp2pgsql-core.c:839:22: runtime error: implicit conversion from type 'int' of value -1 (32-bit, signed) to type 'DBFFieldType' changed the value to 4294967295 (32-bit, unsigned)
@Algunenano
LWGEOM_removepoint: Undefined-behavior in implicit conversion
e6cceb9 
 runtime error: implicit conversion from type 'int32' (aka 'int') of value -1 (32-bit, signed) to type 'uint32' (aka 'unsigned int') changed the value to 4294967295 (32-bit, unsigned)
 UndefinedBehaviorSanitizer: undefined-behavior lwgeom_functions_basic.c:2237:10 in
@Algunenano
ptarray_reverse_in_place: Avoid undefined-behavior
dd03be2 
 runtime error: implicit conversion from type 'unsigned int' of value 4294967295 (32-bit, unsigned) to type 'int' changed the value to -1 (32-bit, signed)
 UndefinedBehaviorSanitizer: undefined-behavior ptarray.c:333:13 in
@Algunenano
LWGEOM_setpoint_linestring: Implicit conversion undef behaviour
30c01e6 
 runtime error: implicit conversion from type 'int32' (aka 'int') of value -1 (32-bit, signed) to type 'unsigned int' changed the value to 4294967295 (32-bit, unsigned)
 UndefinedBehaviorSanitizer: undefined-behavior lwgeom_functions_basic.c:2310:11 in
@Algunenano
loader cunit: Add missing $(CFLAGS) to the main binary
7b3c357
@Algunenano
Change remaining srid int usages to int32_t
54feb82
@Algunenano
Topology: Use uint64 to handle sizes instead of int
e3f205e 
Related to https://trac.osgeo.org/postgis/ticket/4141
@Algunenano
Copy link
Member Author

I'm breaking this into several commits / PRs

@Algunenano Algunenano closed this Apr 24, 2019
@Algunenano Algunenano deleted the msan_buildarea branch November 15, 2019 15:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Komzpa Komzpa Komzpa approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
2 participants
@Algunenano @Komzpa