Jtv/dev - Githubissues

viega commented 2 months ago

Cleaned up GC code and made it so that, in most places, we only scan memory for pointers that represent fields with pointers. This basically is handled by per-datatype collection hooks, and did a bit of work on just the primary hatrack dictionary to support (per-datatype values that get passed through during record allocation).
Module importing is done, and the system __init.c4m is automatically imported (which currently imports one starting module).
Added C backtraces, including in a signal handler if there's a SEGV.
Small improvements to the test command line, such as searching your cwd for files if the file name isn't found in the test dir.
Got a bunch more tests working.

The pattern stuff is a bit broken due to changes; there are some work-arounds there. I don't plan to fight variadic macros right now, so the workarounds are going to stay for a while.

viega commented 2 months ago

Yup that got it thanks.

viega commented 2 months ago

Well, I didn't realize I did the same thing in two places. But your crash has nothing to do with that; it's a file not found error. Hopefully fixed the message on that too.

ee7 commented 2 months ago

it's a file not found error.

OK, thanks. After changing locally:

- export CON4M_TEST_DIR='/path/to/libcon4m/tests'
+ export CON4M_ROOT='/path/to/libcon4m'

with abfb442e17f9598b53701c394757097b2aa78b69, the tests pass ony my machine, and ASan output is back to the (to my understanding) expected:

==37019==ERROR: AddressSanitizer: stack-use-after-return on address 0x7603c990b6a0 at pc 0x611d9efb1f05 bp 0x7ffc782ec280 sp 0x7ffc782ec278
READ of size 8 at 0x7603c990b6a0 thread T0
SCARINESS: 61 (8-byte-read-stack-use-after-return)
    #0 0x611d9efb1f04 in scan_range_for_allocs libcon4m/build/../src/con4m/collect.c:560:37
    #1 0x611d9efb1e8f in scan_roots libcon4m/build/../src/con4m/collect.c:600:9
    #2 0x611d9efb14dc in raw_trace libcon4m/build/../src/con4m/collect.c:638:5
    #3 0x611d9efb0201 in c4m_collect_arena libcon4m/build/../src/con4m/collect.c:981:5
    #4 0x611d9efb1921 in c4m_gc_thread_collect libcon4m/build/../src/con4m/collect.c:1101:24
    #5 0x611d9ef96c98 in main libcon4m/build/../src/tests/test.c:767:5
    #6 0x7603cb94ec87  (/usr/lib/libc.so.6+0x25c87) (BuildId: 32a656aa5562eece8c59a585f5eacd6cf5e2307b)
    #7 0x7603cb94ed4b in __libc_start_main (/usr/lib/libc.so.6+0x25d4b) (BuildId: 32a656aa5562eece8c59a585f5eacd6cf5e2307b)
    #8 0x611d9ee5f3b4 in _start (libcon4m/build/c4test+0x6e3b4) (BuildId: c03daa209a2f66f96e5de78168d8a61d758c73c5)

Address 0x7603c990b6a0 is located in stack of thread T0 at offset 32 in frame
    #0 0x611d9efb3bdf in c4m_pass_kargs libcon4m/build/../src/con4m/kargs.c:60

  This frame has 1 object(s):
    [32, 56) 'args' (line 61) <== Memory access at offset 32 is inside this variable
HINT: this may be a false positive if your program uses some custom stack unwind mechanism, swapcontext or vfork
      (longjmp and C++ exceptions *are* supported)
SUMMARY: AddressSanitizer: stack-use-after-return libcon4m/build/../src/con4m/collect.c:560:37 in scan_range_for_allocs

Should we act on the below new UBSan output?

../src/con4m/types.c:272:54: runtime error: member access within misaligned address 0x5d76f3d76368 for type 'c4m_alloc_hdr' (aka 'struct c4m_alloc_hdr'), which requires 16 byte alignment
0x5d76f3d76368: note: pointer points here
 00 00 00 00  00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00  00 00 00 00
              ^ 
    #0 0x5d76e30acfc4 in c4m_early_alloc_type libcon4m/build/../src/con4m/types.c:272:54
    #1 0x5d76e30a7470 in setup_primitive_types libcon4m/build/../src/con4m/types.c:1683:30
    #2 0x5d76e30a7431 in c4m_initialize_global_types libcon4m/build/../src/con4m/types.c:1724:5
    #3 0x5d76e30d936d in c4m_init libcon4m/build/../src/con4m/init.c:292:5
    #4 0x7c12ee239dc3 in __libc_start_main (/usr/lib/libc.so.6+0x25dc3) (BuildId: 32a656aa5562eece8c59a585f5eacd6cf5e2307b)
    #5 0x5d76e2ff3674 in _start (libcon4m/build/c4test+0x131674) (BuildId: 38106552160dee4dec026c357c823d84cc186ef5)

SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior ../src/con4m/types.c:272:54 
../src/con4m/types.c:277:10: runtime error: member access within misaligned address 0x5d76f3d76368 for type 'c4m_alloc_hdr' (aka 'struct c4m_alloc_hdr'), which requires 16 byte alignment
0x5d76f3d76368: note: pointer points here
 00 00 00 00  00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00  00 00 00 00
              ^ 
    #0 0x5d76e30ad099 in c4m_early_alloc_type libcon4m/build/../src/con4m/types.c:277:10
    #1 0x5d76e30a7470 in setup_primitive_types libcon4m/build/../src/con4m/types.c:1683:30
    #2 0x5d76e30a7431 in c4m_initialize_global_types libcon4m/build/../src/con4m/types.c:1724:5
    #3 0x5d76e30d936d in c4m_init libcon4m/build/../src/con4m/init.c:292:5
    #4 0x7c12ee239dc3 in __libc_start_main (/usr/lib/libc.so.6+0x25dc3) (BuildId: 32a656aa5562eece8c59a585f5eacd6cf5e2307b)
    #5 0x5d76e2ff3674 in _start (libcon4m/build/c4test+0x131674) (BuildId: 38106552160dee4dec026c357c823d84cc186ef5)

SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior ../src/con4m/types.c:277:10 
../src/con4m/types.c:293:53: runtime error: member access within misaligned address 0x5d76f3d78c88 for type 'c4m_alloc_hdr' (aka 'struct c4m_alloc_hdr'), which requires 16 byte alignment
0x5d76f3d78c88: note: pointer points here
 00 00 00 00  00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00  00 00 00 00
              ^ 
    #0 0x5d76e30ad503 in early_type_list libcon4m/build/../src/con4m/types.c:293:53
    #1 0x5d76e30a7bc9 in setup_list_of_types libcon4m/build/../src/con4m/types.c:319:34
    #2 0x5d76e30a7436 in c4m_initialize_global_types libcon4m/build/../src/con4m/types.c:1725:5
    #3 0x5d76e30d936d in c4m_init libcon4m/build/../src/con4m/init.c:292:5
    #4 0x7c12ee239dc3 in __libc_start_main (/usr/lib/libc.so.6+0x25dc3) (BuildId: 32a656aa5562eece8c59a585f5eacd6cf5e2307b)
    #5 0x5d76e2ff3674 in _start (libcon4m/build/c4test+0x131674) (BuildId: 38106552160dee4dec026c357c823d84cc186ef5)

SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior ../src/con4m/types.c:293:53 
../src/con4m/types.c:296:10: runtime error: member access within misaligned address 0x5d76f3d78c88 for type 'c4m_alloc_hdr' (aka 'struct c4m_alloc_hdr'), which requires 16 byte alignment
0x5d76f3d78c88: note: pointer points here
 00 00 00 00  00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00  00 00 00 00
              ^ 
    #0 0x5d76e30ad583 in early_type_list libcon4m/build/../src/con4m/types.c:296:10
    #1 0x5d76e30a7bc9 in setup_list_of_types libcon4m/build/../src/con4m/types.c:319:34
    #2 0x5d76e30a7436 in c4m_initialize_global_types libcon4m/build/../src/con4m/types.c:1725:5
    #3 0x5d76e30d936d in c4m_init libcon4m/build/../src/con4m/init.c:292:5
    #4 0x7c12ee239dc3 in __libc_start_main (/usr/lib/libc.so.6+0x25dc3) (BuildId: 32a656aa5562eece8c59a585f5eacd6cf5e2307b)
    #5 0x5d76e2ff3674 in _start (libcon4m/build/c4test+0x131674) (BuildId: 38106552160dee4dec026c357c823d84cc186ef5)

SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior ../src/con4m/types.c:296:10

or this new warning?

[54/128] Compiling C object libcon4m.a.p/src_con4m_collect.c.o
../src/con4m/collect.c:146:5: warning: 'pthread_attr_getstackaddr' is deprecated [-Wdeprecated-declarations]
  146 |     pthread_attr_getstackaddr(&attrs, (void **)&addr);
      |     ^
/usr/include/pthread.h:355:33: note: 'pthread_attr_getstackaddr' has been explicitly marked deprecated here
  355 |      __THROW __nonnull ((1, 2)) __attribute_deprecated__;
      |                                 ^
/usr/include/x86_64-linux-gnu/sys/cdefs.h:356:51: note: expanded from macro '__attribute_deprecated__'
  356 | # define __attribute_deprecated__ __attribute__ ((__deprecated__))
      |                                                   ^
1 warning generated.
[...]
[128/128] Linking target c4test
/usr/bin/ld: libcon4m.a.p/src_con4m_collect.c.o: in function `c4m_get_stack_scan_region':
/home/runner/work/libcon4m/libcon4m/cicd/../src/con4m/collect.c:146:(.text+0x14ff): warning: the use of `pthread_attr_getstackaddr' is deprecated, use `pthread_attr_getstack'

due to:

https://github.com/crashappsec/libcon4m/blob/abfb442e17f9598b53701c394757097b2aa78b69/include/con4m.h#L10

ee7 commented 2 months ago

l but three tests fail, though I'm not sure if that's expected or not

I think it is now required to set:

CON4M_ROOT=/path/to/libcon4m/repo/root

orangematt commented 2 months ago

l but three tests fail, though I'm not sure if that's expected or not

I think it is now required to set:
CON4M_ROOT=/path/to/libcon4m/repo/root

That fixed it, thanks. All tests pass

viega commented 2 months ago

I'll check out what's happening when the environment variable isn't set for the runner; I'll make sure it definitely works if you run the dev script, even if the envvar isn't set.

ee7 commented 2 months ago

Regarding the new warning for pthread_attr_getstackaddr, commit 6fe8f43 did:

+#define C4M_USE_FRAME_INTRINSIC

and we had:

https://github.com/crashappsec/libcon4m/blob/7431fd786f455149b2e50b57c49409f8c93bd98b/src/con4m/collect.c#L145-L152

Will confirm the options soon that yield the UBSan alignment warnings.

ee7 commented 2 months ago

Confirming that with:

The new commit on main (https://github.com/crashappsec/libcon4m/commit/7431fd786f455149b2e50b57c49409f8c93bd98b)
Doing nothing but enabling UBSan

Running build/c4test basic01.c4m produces the UBSan errors in https://github.com/crashappsec/libcon4m/pull/85#issuecomment-2228677650 both when:

dev build uses -Duse_memcheck=true (as on main)

dev is edited to remove -Duse_memcheck=true from line 183:

-    build)   meson_build build --buildtype=plain -Duse_memcheck=true
+    build)   meson_build build --buildtype=plain

Build log:

$ git log -1 --oneline
7431fd7 (HEAD -> main, crashappsec/main, crashappsec/HEAD) Granular GC handling; bactraces; support for module importing. (#85)
$ git diff main
--- a/meson.options
+++ b/meson.options
@@ -1,5 +1,5 @@
-option('use_ubsan', type: 'boolean', value: false)
+option('use_ubsan', type: 'boolean', value: true)
$ rm -rf build
$ CC=clang ./dev build
[-- libcon4m --] Creating meson target build
[-- libcon4m --] meson setup build --buildtype=plain -Duse_memcheck=true
The Meson build system
Version: 1.5.0
Source dir: /foo/libcon4m
Build dir: /foo/libcon4m/build
Build type: native build
Project name: libcon4m
Project version: 0.1.0
C compiler for the host machine: clang (clang 18.1.8 "clang version 18.1.8")
C linker for the host machine: clang ld.bfd 2.42.0
Host machine cpu family: x86_64
Host machine cpu: x86_64
Library backtrace found: NO
Checking if "forkpty_check" : links: YES 
Run-time dependency threads found: YES
Library m found: YES
Library ffi found: YES
Library crypto found: YES
Library ssl found: YES
Found pkg-config: YES (/usr/bin/pkg-config) 2.1.1
Run-time dependency libunibreak found: YES 6.1
Run-time dependency libutf8proc found: YES 2.9.0
Library atomic found: YES
Build targets in project: 2

libcon4m 0.1.0

  User defined options
    buildtype   : plain
    use_memcheck: true

Found ninja-1.12.1 at /usr/bin/ninja
[-- libcon4m --] Compiling meson target build                                                                                                                                                                                                                   
INFO: autodetecting backend as ninja
INFO: calculating backend command to run: /usr/bin/ninja
[8/83] Compiling C object libcon4m.a.p/src_con4m_collect.c.o
../src/con4m/collect.c:146:5: warning: 'pthread_attr_getstackaddr' is deprecated [-Wdeprecated-declarations]
  146 |     pthread_attr_getstackaddr(&attrs, (void **)&addr);
      |     ^
/usr/include/pthread.h:355:33: note: 'pthread_attr_getstackaddr' has been explicitly marked deprecated here
  355 |      __THROW __nonnull ((1, 2)) __attribute_deprecated__;
      |                                 ^
/usr/include/sys/cdefs.h:356:51: note: expanded from macro '__attribute_deprecated__'
  356 | # define __attribute_deprecated__ __attribute__ ((__deprecated__))
      |                                                   ^
1 warning generated.
[83/83] Linking target c4test
/usr/bin/ld: libcon4m.a.p/src_con4m_collect.c.o: in function `c4m_get_stack_scan_region':
/foo/libcon4m/build/../src/con4m/collect.c:146:(.text+0x4f2d): warning: the use of `pthread_attr_getstackaddr' is deprecated, use `pthread_attr_getstack'
[-- libcon4m --] Done!

crashappsec / libcon4m

Jtv/dev #85