| Message ID | 157616229728.30610.11942820198797258041.scripted-patch-series@arm.com |
|---|---|
| State | New |
| Headers | show |
| Series | [0/X] HWASAN v3 | expand |
I've noticed a few minor problems with this patch series after I sent it out (mostly testcase stuff, one documentation tidy-up, but also that one patch didn't bootstrap due to something fixed in a later patch). I also rely on a documentation change that isn't part of the series. I figure I should make this easy on anyone that wants to try the patch series out, so I'm attaching a compressed tarfile containing the entire patch series plus the additional documentation patch so it can all be applied at once with `git apply *`. It's attached. Matthew. On 12/12/2019 15:18, Matthew Malcomson wrote: > Hello, > > I've gone through the suggestions Martin made and implemented the ones I think > I can implement for GCC10. > > The two functionality changes in this version are: > Added the --param's hwasan-instrument-reads, hwasan-instrument-writes, > hwasan-instrument-allocas, hwasan-memintrin, options. I.e. Those that asan has > and that make sense for hwasan. > > Avoided HWASAN_STACK_BACKGROUND in hwasan_increment_tag when using a > deterministic tagging approach. > > > There are a lot of extra comments and tests. > > > Bootstrapped and regtested on x86_64 and AArch64. > Bootstrapped with `--with-build-config=bootstrap-hwasan` on AArch64 and hwasan > features tested there. > Built the linux kernel using this feature and ran the test_kasan.ko testing to > check the this works for the kernel. > (NOTE: I actually did all the above testing before a search and replace of > `memory_tagging_p` for `hwasan_sanitize_p` and fixing a typo in the > `hwasan-instrument-allocas` parameter name, I will run all the tests again > before committing but figure I'll send this out now since I fully expect the > tests to still pass). > > > I noticed one extra testsuite failure from those mentioned in the previous > version emails: g++.dg/cpp2a/ucn2.C. > I believe this is HWASAN correctly catching a problem in the compiler. > I've logged the issue here https://gcc.gnu.org/bugzilla/show_bug.cgi?id=92919 . > > > I haven't gotten ASAN_MARK to print as HWASAN_MARK when using memory tagging, > since I'm not sure the way I found to implement this would be acceptable. The > inlined patch below works but it requires a special declaration instead of just > an ~#include~. > > > diff --git a/gcc/internal-fn.h b/gcc/internal-fn.h > index a1bc081..d81eb12 100644 > --- a/gcc/internal-fn.h > +++ b/gcc/internal-fn.h > @@ -101,10 +101,16 @@ extern void init_internal_fns (); > > extern const char *const internal_fn_name_array[]; > > + > +extern bool hwasan_sanitize_p (void); > static inline const char * > internal_fn_name (enum internal_fn fn) > { > - return internal_fn_name_array[(int) fn]; > + const char *ret = internal_fn_name_array[(int) fn]; > + if (! strcmp (ret, "ASAN_MARK") > + && hwasan_sanitize_p ()) > + return "HWASAN_MARK"; > + return ret; > } > > extern internal_fn lookup_internal_fn (const char *); > > > Entire patch series attached to cover letter. >
Ping On 17/12/2019 14:11, Matthew Malcomson wrote: > I've noticed a few minor problems with this patch series after I sent it > out (mostly testcase stuff, one documentation tidy-up, but also that one > patch didn't bootstrap due to something fixed in a later patch). > > I also rely on a documentation change that isn't part of the series. > > I figure I should make this easy on anyone that wants to try the patch > series out, so I'm attaching a compressed tarfile containing the entire > patch series plus the additional documentation patch so it can all be > applied at once with `git apply *`. > > It's attached. > > Matthew. > > > > On 12/12/2019 15:18, Matthew Malcomson wrote: >> Hello, >> >> I've gone through the suggestions Martin made and implemented the ones I think >> I can implement for GCC10. >> >> The two functionality changes in this version are: >> Added the --param's hwasan-instrument-reads, hwasan-instrument-writes, >> hwasan-instrument-allocas, hwasan-memintrin, options. I.e. Those that asan has >> and that make sense for hwasan. >> >> Avoided HWASAN_STACK_BACKGROUND in hwasan_increment_tag when using a >> deterministic tagging approach. >> >> >> There are a lot of extra comments and tests. >> >> >> Bootstrapped and regtested on x86_64 and AArch64. >> Bootstrapped with `--with-build-config=bootstrap-hwasan` on AArch64 and hwasan >> features tested there. >> Built the linux kernel using this feature and ran the test_kasan.ko testing to >> check the this works for the kernel. >> (NOTE: I actually did all the above testing before a search and replace of >> `memory_tagging_p` for `hwasan_sanitize_p` and fixing a typo in the >> `hwasan-instrument-allocas` parameter name, I will run all the tests again >> before committing but figure I'll send this out now since I fully expect the >> tests to still pass). >> >> >> I noticed one extra testsuite failure from those mentioned in the previous >> version emails: g++.dg/cpp2a/ucn2.C. >> I believe this is HWASAN correctly catching a problem in the compiler. >> I've logged the issue here https://gcc.gnu.org/bugzilla/show_bug.cgi?id=92919 . >> >> >> I haven't gotten ASAN_MARK to print as HWASAN_MARK when using memory tagging, >> since I'm not sure the way I found to implement this would be acceptable. The >> inlined patch below works but it requires a special declaration instead of just >> an ~#include~. >> >> >> diff --git a/gcc/internal-fn.h b/gcc/internal-fn.h >> index a1bc081..d81eb12 100644 >> --- a/gcc/internal-fn.h >> +++ b/gcc/internal-fn.h >> @@ -101,10 +101,16 @@ extern void init_internal_fns (); >> >> extern const char *const internal_fn_name_array[]; >> >> + >> +extern bool hwasan_sanitize_p (void); >> static inline const char * >> internal_fn_name (enum internal_fn fn) >> { >> - return internal_fn_name_array[(int) fn]; >> + const char *ret = internal_fn_name_array[(int) fn]; >> + if (! strcmp (ret, "ASAN_MARK") >> + && hwasan_sanitize_p ()) >> + return "HWASAN_MARK"; >> + return ret; >> } >> >> extern internal_fn lookup_internal_fn (const char *); >> >> >> Entire patch series attached to cover letter. >>
On 12/12/19 4:18 PM, Matthew Malcomson wrote: Hello. I've just sent few comments that are related to the v3 of the patch set. Based on the HWASAN (limited) knowledge the patch seems reasonable to me. I haven't looked much at the newly introduced RTL-hooks. But these seems to me isolated to the aarch64 port. I can also verify that the patchset works on my aarch64 linux machine and hwasan.exp and asan.exp tests succeed. > I haven't gotten ASAN_MARK to print as HWASAN_MARK when using memory tagging, > since I'm not sure the way I found to implement this would be acceptable. The > inlined patch below works but it requires a special declaration instead of just > an ~#include~. Knowing that, I would not bother with the printing of HWASAN_MARK. Thanks for the series, Martin
Hi everyone, I'm writing this email to summarise & publicise the state of this patch series, especially the difficulties around approval for GCC 10 mentioned on IRC. The main obstacle seems to be that no maintainer feels they have enough knowledge about hwasan and justification that it's worthwhile to approve the patch series. Similarly, Martin has given a review of the parts of the code he can (thanks!), but doesn't feel he can do a deep review of the code related to the RTL hooks and stack expansion -- hence that part is as yet not reviewed in-depth. The questions around justification raised on IRC are mainly that it seems like a proof-of-concept for MTE rather than a stand-alone useable sanitizer. Especially since in the GNU world hwasan instrumented code is not really ready for production since we can only use the less-"interceptor ABI" rather than the "platform ABI". This restriction is because there is no version of glibc with the required modifications to provide the "platform ABI". (n.b. that since https://reviews.llvm.org/D69574 the code-generation for these ABI's is the same). From my perspective the reasons that make HWASAN useful in itself are: 1) Much less memory usage. From a back-of-the-envelope calculation based on the hwasan paper's table of memory overhead from over-alignment https://arxiv.org/pdf/1802.09517.pdf I guess hwasan instrumented code has an overhead of about 1.1x (~4% from overalignment and ~6.25% from shadow memory), while asan seems to have an overhead somewhere in the range 1.5x - 3x. Maybe there's some data out there comparing total overheads that I haven't found? (I'd appreciate a reference if anyone has that info). 2) Available on more architectures that MTE. HWASAN only requires TBI, which is a feature of all AArch64 machines, while MTE will be an optional extension and only available on certain architectures. 3) This enables using hwasan in the kernel. While instrumented user-space applications will be using the "interceptor ABI" and hence are likely not production-quality, the biggest aim of implementing hwasan in GCC is to allow building the Linux kernel with tag-based sanitization using GCC. Instrumented kernel code uses hooks in the kernel itself, so this ABI distinction is no longer relevant, and this sanitizer should produce a production-quality kernel binary. I'm hoping I can find a maintainer willing to review and ACK this patch series -- especially with stage3 coming to a close soon. If there's anything else I could do to help get someone willing up-to-speed then please just ask. Cheers, Matthew On 07/01/2020 15:14, Martin Liška wrote: > On 12/12/19 4:18 PM, Matthew Malcomson wrote: > > Hello. > > I've just sent few comments that are related to the v3 of the patch set. > Based on the HWASAN (limited) knowledge the patch seems reasonable to me. > I haven't looked much at the newly introduced RTL-hooks. > But these seems to me isolated to the aarch64 port. > > I can also verify that the patchset works on my aarch64 linux machine and > hwasan.exp and asan.exp tests succeed. > >> I haven't gotten ASAN_MARK to print as HWASAN_MARK when using memory >> tagging, >> since I'm not sure the way I found to implement this would be >> acceptable. The >> inlined patch below works but it requires a special declaration >> instead of just >> an ~#include~. > > Knowing that, I would not bother with the printing of HWASAN_MARK. > > Thanks for the series, > Martin
[asan/hwasan co-author here, with clearly biased opinions] On Android, HWASAN is already a fully usable testing tool. We apply it to the kernel, user space system libraries, and select apps. A phone with HWASAN-ified system is fully usable (I carry one as my primary device since March 2019). HWASAN has discovered over 120 bugs by now (heap-use-after-free, heap/stack buffer overflows, stack-use-after-return, double free). Many of the bugs were discovered during the everyday use (as opposed to testing in the lab). The overhead is low enough that on a top-tier CPU the user will rarely notice any slowdown (the increased battery drain *is* noticeable - compiler instrumentation is not a substitute for hardware). HWASAN has also helped discover 4 instances of future incompatibility with MTE, all fixed. The main benefit of HWASAN over ASAN is, as Matthew correctly explains, the memory usage. On embedded devices, this is often the difference between "can't deploy" and "can deploy" because, unlike in the server land, you can't install more RAM. The other, more subtle benefit, is that HWASAN is more sensitive to some types of bugs, such as buffer-overflow-far-from-bounds or use-after-long-ago-free, etc. MTE hardware is years away. Even once we have it in major new devices, many smaller devices will still be running on Arm v8, for a decade or two. As with ASAN/TSAN/UBSAN, having this sanitizer implemented in GCC will vastly extend its user base and applicability and thus contribute to the overall code quality and security. Whether HWASAN should intercept libc functions or libc itself should support HWASAN... My strong opinion is that today the interception approach can only be seen as a way to prototype. ASAN, implemented in 2011, had to use interception because we needed to get a new idea working fast. However, over these 9 years, the interception caused an enormous amount of complexity and user dissatisfaction. The Android implementation of HWASAN (with hooks in the Bionic libc and no interceptors) is many times simpler, robust, and complete. We need to do the same for other LIBCs, eventually, but we don't have to do it immediately. --kcc On Wed, Jan 8, 2020 at 3:26 AM Matthew Malcomson <Matthew.Malcomson@arm.com> wrote: > > Hi everyone, > > I'm writing this email to summarise & publicise the state of this patch > series, especially the difficulties around approval for GCC 10 mentioned > on IRC. > > > The main obstacle seems to be that no maintainer feels they have enough > knowledge about hwasan and justification that it's worthwhile to approve > the patch series. > > Similarly, Martin has given a review of the parts of the code he can > (thanks!), but doesn't feel he can do a deep review of the code related > to the RTL hooks and stack expansion -- hence that part is as yet not > reviewed in-depth. > > > > The questions around justification raised on IRC are mainly that it > seems like a proof-of-concept for MTE rather than a stand-alone useable > sanitizer. Especially since in the GNU world hwasan instrumented code > is not really ready for production since we can only use the > less-"interceptor ABI" rather than the "platform ABI". This restriction > is because there is no version of glibc with the required modifications > to provide the "platform ABI". > > (n.b. that since https://reviews.llvm.org/D69574 the code-generation for > these ABI's is the same). > > > From my perspective the reasons that make HWASAN useful in itself are: > > 1) Much less memory usage. > > From a back-of-the-envelope calculation based on the hwasan paper's > table of memory overhead from over-alignment > https://arxiv.org/pdf/1802.09517.pdf I guess hwasan instrumented code > has an overhead of about 1.1x (~4% from overalignment and ~6.25% from > shadow memory), while asan seems to have an overhead somewhere in the > range 1.5x - 3x. > > Maybe there's some data out there comparing total overheads that I > haven't found? (I'd appreciate a reference if anyone has that info). > > > > 2) Available on more architectures that MTE. > > HWASAN only requires TBI, which is a feature of all AArch64 machines, > while MTE will be an optional extension and only available on certain > architectures. > > > 3) This enables using hwasan in the kernel. > > While instrumented user-space applications will be using the > "interceptor ABI" and hence are likely not production-quality, the > biggest aim of implementing hwasan in GCC is to allow building the Linux > kernel with tag-based sanitization using GCC. > > Instrumented kernel code uses hooks in the kernel itself, so this ABI > distinction is no longer relevant, and this sanitizer should produce a > production-quality kernel binary. > > > > > I'm hoping I can find a maintainer willing to review and ACK this patch > series -- especially with stage3 coming to a close soon. If there's > anything else I could do to help get someone willing up-to-speed then > please just ask. > > > Cheers, > Matthew > > > > On 07/01/2020 15:14, Martin Liška wrote: > > On 12/12/19 4:18 PM, Matthew Malcomson wrote: > > > > Hello. > > > > I've just sent few comments that are related to the v3 of the patch set. > > Based on the HWASAN (limited) knowledge the patch seems reasonable to me. > > I haven't looked much at the newly introduced RTL-hooks. > > But these seems to me isolated to the aarch64 port. > > > > I can also verify that the patchset works on my aarch64 linux machine and > > hwasan.exp and asan.exp tests succeed. > > > >> I haven't gotten ASAN_MARK to print as HWASAN_MARK when using memory > >> tagging, > >> since I'm not sure the way I found to implement this would be > >> acceptable. The > >> inlined patch below works but it requires a special declaration > >> instead of just > >> an ~#include~. > > > > Knowing that, I would not bother with the printing of HWASAN_MARK. > > > > Thanks for the series, > > Martin >
On 1/8/20 11:26 AM, Matthew Malcomson wrote: > Hi everyone, > > I'm writing this email to summarise & publicise the state of this patch > series, especially the difficulties around approval for GCC 10 mentioned > on IRC. > > > The main obstacle seems to be that no maintainer feels they have enough > knowledge about hwasan and justification that it's worthwhile to approve > the patch series. > > Similarly, Martin has given a review of the parts of the code he can > (thanks!), but doesn't feel he can do a deep review of the code related > to the RTL hooks and stack expansion -- hence that part is as yet not > reviewed in-depth. > > > > The questions around justification raised on IRC are mainly that it > seems like a proof-of-concept for MTE rather than a stand-alone useable > sanitizer. Especially since in the GNU world hwasan instrumented code > is not really ready for production since we can only use the > less-"interceptor ABI" rather than the "platform ABI". This restriction > is because there is no version of glibc with the required modifications > to provide the "platform ABI". > > (n.b. that since https://reviews.llvm.org/D69574 the code-generation for > these ABI's is the same). > > > From my perspective the reasons that make HWASAN useful in itself are: > > 1) Much less memory usage. > > From a back-of-the-envelope calculation based on the hwasan paper's > table of memory overhead from over-alignment > https://arxiv.org/pdf/1802.09517.pdf I guess hwasan instrumented code > has an overhead of about 1.1x (~4% from overalignment and ~6.25% from > shadow memory), while asan seems to have an overhead somewhere in the > range 1.5x - 3x. > > Maybe there's some data out there comparing total overheads that I > haven't found? (I'd appreciate a reference if anyone has that info). > > > > 2) Available on more architectures that MTE. > > HWASAN only requires TBI, which is a feature of all AArch64 machines, > while MTE will be an optional extension and only available on certain > architectures. > > > 3) This enables using hwasan in the kernel. > > While instrumented user-space applications will be using the > "interceptor ABI" and hence are likely not production-quality, the > biggest aim of implementing hwasan in GCC is to allow building the Linux > kernel with tag-based sanitization using GCC. > > Instrumented kernel code uses hooks in the kernel itself, so this ABI > distinction is no longer relevant, and this sanitizer should produce a > production-quality kernel binary. > > > > > I'm hoping I can find a maintainer willing to review and ACK this patch > series -- especially with stage3 coming to a close soon. If there's > anything else I could do to help get someone willing up-to-speed then > please just ask. > FWIW I've reviewed the aarch64 parts over the lifetime of the patch series and I am okay with them. Given the reviews of the sanitiser, library and aarch64 backend components, and the data at https://gcc.gnu.org/ml/gcc-patches/2020-01/msg00387.html how can we move forward with commit approval ? Is this something a global reviewer can help with, Jeff ? :) Thanks, Kyrill > > Cheers, > Matthew > > > > On 07/01/2020 15:14, Martin Liška wrote: > > On 12/12/19 4:18 PM, Matthew Malcomson wrote: > > > > Hello. > > > > I've just sent few comments that are related to the v3 of the patch set. > > Based on the HWASAN (limited) knowledge the patch seems reasonable > to me. > > I haven't looked much at the newly introduced RTL-hooks. > > But these seems to me isolated to the aarch64 port. > > > > I can also verify that the patchset works on my aarch64 linux > machine and > > hwasan.exp and asan.exp tests succeed. > > > >> I haven't gotten ASAN_MARK to print as HWASAN_MARK when using memory > >> tagging, > >> since I'm not sure the way I found to implement this would be > >> acceptable. The > >> inlined patch below works but it requires a special declaration > >> instead of just > >> an ~#include~. > > > > Knowing that, I would not bother with the printing of HWASAN_MARK. > > > > Thanks for the series, > > Martin >
Hello,
This is v4 of the HWASAN patches which add the LLVM hardware address
sanitizer (HWASAN) to GCC.
The document describing HWASAN can be found here
http://clang.llvm.org/docs/HardwareAssistedAddressSanitizerDesign.html.
This address sanitizer only works for AArch64 at the moment. It
requires the "top byte ignore" feature where the top byte of a pointer
does not affect dereferences. This is checked for by a backend hook so
that if other architectures have this feature HWASAN can be used for
them.
We require a linux kernel with the relaxed ABI to allow tagged pointers
in system calls. This is in the linux mainline, I have been testing
this feature on 5.8.0, but it has been in since at least 5.5.0.
HWASAN works by storing a tag in the top bits of every pointer and a tag in
a shadow memory region corresponding to each area of memory pointed at.
On every memory access through a pointer the tag in the pointer is
checked against the tag in shadow memory corresponding to the memory the
pointer is accessing. If the pointer tag and memory tag do not match
then a fault is signalled.
The instrumentation required for this sanitizer has a large overlap with
the instrumentation required for implementing MTE (which has similar
functionality but checks are automatically done in the hardware and
instructions for tagging shadow memory and for managing tags are
provided by the architecture).
https://community.arm.com/developer/ip-products/processors/b/processors-ip-blog/posts/arm-a-profile-architecture-2018-developments-armv85a
We hope to use the HWASAN framework to implement MTE for the stack, so I
have included a "dummy" patch showing how this might be done in the full
patch set attached to this cover letter. (mte-approach.patch)
Mainly this is the same patch series as posted in December rebased onto
master, but I have also identified and fixed a few minor bugs.
- A few bugs around the use of ptr_mode and Pmode
(for AArch64 only observable with -mabi=ilp32).
- A frame-extent calculation bug when every stack object in a frame is
`large aligned` -- see the arguments that hwasan_emit_prologue
passes to hwasan_emit_untag_frame.
- A few changes of parameter names to match Clang.
The names were originally chosen to match ASAN, but I found that
most of these parameters can be mapped to a Clang configurable and
that renaming them to match clang makes Makefiles (like
scripts/Makefile.kasan in the linux kernel) easier to write.
Bootstrapped and regtested on x86_64 and AArch64.
Bootstrapped with `--with-build-config=bootstrap-hwasan` on AArch64 and
hwasan features tested there (all new regression failures accounted for
manually).
Built the linux kernel using this feature and ran the test_kasan.ko
testing to check the this works for the kernel.
(NOTE: Stack-tagging for the linux kernel has recently been added for
clang. Testing GCC stack-tagging on the kernel showed two small bugs --
1) using __hwasan_tag_pointer for allocas which isn't available in the
kernel, 2) not avoiding zero tag due to the stack pointer having tag of
0xff. I'm working on fixing these but I'm sending up the patch series
as-is to get feedback earlier. A small change to a kernel makefile, and
some headers are needed to build it with this sanitizer -- a patch with
the changes and a text file explaining how to build and test the kernel
with HWASAN is attached.
linux-for-gcc-hwasan.patch and testing-kasan.txt )
Last time the patch set went up Martin Liska had provided a patch to
build upon that added libhwasan into the libsanitizer directory so the
patch set didn't contain anything to do that.
The "full" patchset attached to this email contains a patch that adds
this library so that anyone who wants to test this can do so, but there
is no corresponding email for the individual patch.
(introduce-libhwasan.patch)
NOTE:
1) The target of having this sanitizer in GCC is for use on the Linux
kernel. The implementation should be good for the kernel, while the
userspace story is not robust.
The main use case of HWASAN is when it lies *underneath* the system
libc. In this approach the libc calls into libhwasan on important
events (e.g. longjmp).
This has been used very well in Android.
At the moment there are no plans to do similar on distros with a
modified glibc.
A userspace story can be made by intercepting important functions.
LLVM maintain this approach for *testing* only, and hence it is not of
a production quality.
Similarly we aim to use this "interception" model for testing, while
maintaining the focus of the GCC port of use on the kernel.
2) This sanitizer has no handling of C++ exceptions.
If an exception is thrown the "shadow stack" is left in an
inconsistent state and will likely eventually cause a false positive
later on in the program.
This is due to the fact that the handling of exceptions in LLVM relies
on having the frame record appear after any locals(*). This
restriction is not satisfied by GCC due to its frame layout
optimisation.
(*) https://github.com/llvm-mirror/compiler-rt/blob/master/lib/hwasan/hwasan_exceptions.cpp#L52
Entire patch set attached to this cover letter.
Adding hwasan tests. Only interesting thing here is that we have to make sure the tagging mechanism is deterministic to avoid flaky tests. gcc/testsuite/ChangeLog: * c-c++-common/hwasan/aligned-alloc.c: New test. * c-c++-common/hwasan/alloca-array-accessible.c: New test. * c-c++-common/hwasan/alloca-gets-different-tag.c: New test. * c-c++-common/hwasan/alloca-outside-caught.c: New test. * c-c++-common/hwasan/arguments.c: New test. * c-c++-common/hwasan/arguments-1.c: New test. * c-c++-common/hwasan/arguments-2.c: New test. * c-c++-common/hwasan/arguments-3.c: New test. * c-c++-common/hwasan/asan-pr63316.c: New test. * c-c++-common/hwasan/asan-pr70541.c: New test. * c-c++-common/hwasan/asan-pr78106.c: New test. * c-c++-common/hwasan/asan-pr79944.c: New test. * c-c++-common/hwasan/asan-rlimit-mmap-test-1.c: New test. * c-c++-common/hwasan/bitfield-1.c: New test. * c-c++-common/hwasan/bitfield-2.c: New test. * c-c++-common/hwasan/builtin-special-handling.c: New test. * c-c++-common/hwasan/check-interface.c: New test. * c-c++-common/hwasan/halt_on_error-1.c: New test. * c-c++-common/hwasan/heap-overflow.c: New test. * c-c++-common/hwasan/hwasan-poison-optimisation.c: New test. * c-c++-common/hwasan/hwasan-thread-access-parent.c: New test. * c-c++-common/hwasan/hwasan-thread-basic-failure.c: New test. * c-c++-common/hwasan/hwasan-thread-clears-stack.c: New test. * c-c++-common/hwasan/hwasan-thread-success.c: New test. * c-c++-common/hwasan/kernel-defaults.c: New test. * c-c++-common/hwasan/large-aligned-0.c: New test. * c-c++-common/hwasan/large-aligned-1.c: New test. * c-c++-common/hwasan/large-aligned-untagging-0.c: New test. * c-c++-common/hwasan/large-aligned-untagging-1.c: New test. * c-c++-common/hwasan/large-aligned-untagging-2.c: New test. * c-c++-common/hwasan/large-aligned-untagging-3.c: New test. * c-c++-common/hwasan/large-aligned-untagging-4.c: New test. * c-c++-common/hwasan/large-aligned-untagging-5.c: New test. * c-c++-common/hwasan/large-aligned-untagging-6.c: New test. * c-c++-common/hwasan/large-aligned-untagging-7.c: New test. * c-c++-common/hwasan/macro-definition.c: New test. * c-c++-common/hwasan/no-sanitize-attribute.c: New test. * c-c++-common/hwasan/param-instrument-reads-and-writes.c: New test. * c-c++-common/hwasan/param-instrument-reads.c: New test. * c-c++-common/hwasan/param-instrument-writes.c: New test. * c-c++-common/hwasan/param-instrument-mem-intrinsics.c: New test. * c-c++-common/hwasan/random-frame-tag.c: New test. * c-c++-common/hwasan/sanity-check-pure-c.c: New test. * c-c++-common/hwasan/setjmp-longjmp-0.c: New test. * c-c++-common/hwasan/setjmp-longjmp-1.c: New test. * c-c++-common/hwasan/stack-tagging-basic-0.c: New test. * c-c++-common/hwasan/stack-tagging-basic-1.c: New test. * c-c++-common/hwasan/stack-tagging-disable.c: New test. * c-c++-common/hwasan/unprotected-allocas-0.c: New test. * c-c++-common/hwasan/unprotected-allocas-1.c: New test. * c-c++-common/hwasan/use-after-free.c: New test. * c-c++-common/hwasan/vararray-outside-caught.c: New test. * c-c++-common/hwasan/vararray-stack-restore-correct.c: New test. * c-c++-common/hwasan/very-large-objects.c: New test. * g++.dg/hwasan/hwasan.exp: New file. * g++.dg/hwasan/rvo-handled.C: New test. * gcc.dg/hwasan/hwasan.exp: New file. * gcc.dg/hwasan/nested-functions-0.c: New test. * gcc.dg/hwasan/nested-functions-1.c: New test. * gcc.dg/hwasan/nested-functions-2.c: New test. * lib/hwasan-dg.exp: New file.
Hello. I've just merged libsanitizer and there's the corresponding part that includes libhwasan. Martin
On 10/16/20 11:03 AM, Martin Liška wrote: > Hello. > > I've just merged libsanitizer and there's the corresponding part that includes > libhwasan. > > Martin Hey. I've just made last merge from upstream, there's corresponding hwasan part. Martin
Hi there, Thanks for the heads-up. As it turns out the most recent `libhwasan` crashes when displaying an address on the stack in Linux. I'm currently working on getting it fixed here https://reviews.llvm.org/D91344#2393371 . If this hwasan patch series gets approved and if that patch goes in would it be feasible to bump the libsanitizer merge to whatever version that would be? If not (maybe because stage1 would be finished?) then could/would we end up using the LOCAL_PATCHES approach? Thanks, Matthew
On 11/13/20 5:57 PM, Matthew Malcomson wrote: > Hi there, > > Thanks for the heads-up. > As it turns out the most recent `libhwasan` crashes when displaying an address on the stack in Linux. Hello. What a bad luck. > > I'm currently working on getting it fixed here https://reviews.llvm.org/D91344#2393371 <https://reviews.llvm.org/D91344#2393371> . > If this hwasan patch series gets approved and if that patch goes in would it be feasible to bump the libsanitizer merge to whatever version that would be? > > If not (maybe because stage1 would be finished?) then could/would we end up using the LOCAL_PATCHES approach? Since now, I would prefer doing cherry picks. Hopefully, we'll end just with couple of patches. Thanks, Martin > > Thanks, > Matthew > ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------ > *From:* Martin Liška <mliska@suse.cz> > *Sent:* 13 November 2020 16:33 > *To:* Matthew Malcomson <Matthew.Malcomson@arm.com>; gcc-patches@gcc.gnu.org <gcc-patches@gcc.gnu.org> > *Cc:* jakub@redhat.com <jakub@redhat.com>; Richard Earnshaw <Richard.Earnshaw@arm.com>; kcc@google.com <kcc@google.com>; dodji@redhat.com <dodji@redhat.com>; joseph@codesourcery.com <joseph@codesourcery.com> > *Subject:* Re: [Patch 0/X] HWASAN v4 > On 10/16/20 11:03 AM, Martin Li�ka wrote: >> Hello. >> >> I've just merged libsanitizer and there's the corresponding part that includes >> libhwasan. >> >> Martin > > Hey. > > I've just made last merge from upstream, there's corresponding hwasan part. > > Martin
Hello, Bootstrapped and regression tested on AArch64 (have not finished running tests on the Linux Kernel yet). Sending upstream to catch next set of comments early. This version mainly just consists of requested changes. Some notable exceptions: Have rebased onto a newer version, there was a change to `align_local_variable` that meant some merge conflict resolutions were needed (though no functional change expected). Alignment of stack after object: In Patch 5/X (for tagging objects on the stack) I realised that in some code paths the current method was not ensuring alignment of the stack *after* the object in memory for !FRAME_GROWS_DOWNWARDS. There is a comment in `expand_stack_vars`, alongside the update. Originally we used `align_frame_offset` before space for the object gets allocated to ensure the stack is aligned after the object, and `alloc_stack_frame_space` with relevant alignment to ensure the stack is aligned before the object. When !FRAME_GROWS_DOWNWARDS, using `align_frame_offset` before the object space is allocated does not ensure alignment of the next object. Rather we must use `align_frame_offset` after space is allocated. I now use `align_frame_offset` before *and* after `alloc_stack_frame_space`. This looks nicer than putting if conditions based on FRAME_GROWS_DOWNWARDS and does not end up allocating any extra space than only calling the align function once (it just means the alignment from the `alloc_stack_frame_space` is superfluous). I still ensure `align_local_variable` returns the correct alignment, even though it makes no functional difference. This seems less likely to cause confusion to me. Similarly, this function still uses `SET_DECL_ALIGN` to correctly advertise the alignment that the stack variable is aligned to. (N.b. testing does show that leaving `align_local_variable` as it is doesn't break anything, which is as expected since the alignment is ensured by the calls to `align_frame_offset`). Ensure Alloca causes hwasan frame base to get initialised. In the last version of this patch `expand_HWASAN_CHOOSE_TAG` would use the hwasan frame base using `hwasan_base`. Since this function is used after `expand_stack_vars` and hence after `hwasan_emit_prologue` that would not cause the initialisation of the frame base to be emitted, but would just use the pseudo register without any initialisation. This patch records that initialisation should be emitted when creating the `HWASAN_CHOOSE_TAG` internal function -- so that even if there are no statically allocated local variables the hwasan frame base will be initialisied. New test was introduced to catch this. Introduce HWASAN_SET_TAG internal function. This was something that popped up during testing the linux kernel with stack-tagging. The __hwasan_tag_pointer interface exported by libhwasan is not available in the kernel. Since the main target of putting this feature in GCC is for the kernel we deal with this by emitting code inline to set a tag in a pointer. New test to ensure HWASAN_MARK can handle poly_int sizes. The code is designed to handle any variable sized object, but the only sizes that could actually be used happen to be poly_int sized variables. This is essentially the testcase from pr97696. HWASAN can handle such size types (though ASAN can't) so this should pass for HWASAN. Thanks, Matthew Entire patch series attached to cover letter.
Adding hwasan tests. Only interesting thing here is that we have to make sure the tagging mechanism is deterministic to avoid flaky tests. gcc/testsuite/ChangeLog: * c-c++-common/ubsan/sanitize-recover-7.c: * c-c++-common/hwasan/aligned-alloc.c: New test. * c-c++-common/hwasan/alloca-array-accessible.c: New test. * c-c++-common/hwasan/alloca-base-init.c: New test. * c-c++-common/hwasan/alloca-gets-different-tag.c: New test. * c-c++-common/hwasan/alloca-outside-caught.c: New test. * c-c++-common/hwasan/arguments-1.c: New test. * c-c++-common/hwasan/arguments-2.c: New test. * c-c++-common/hwasan/arguments-3.c: New test. * c-c++-common/hwasan/arguments.c: New test. * c-c++-common/hwasan/asan-pr63316.c: New test. * c-c++-common/hwasan/asan-pr70541.c: New test. * c-c++-common/hwasan/asan-pr78106.c: New test. * c-c++-common/hwasan/asan-pr79944.c: New test. * c-c++-common/hwasan/asan-rlimit-mmap-test-1.c: New test. * c-c++-common/hwasan/bitfield-1.c: New test. * c-c++-common/hwasan/bitfield-2.c: New test. * c-c++-common/hwasan/builtin-special-handling.c: New test. * c-c++-common/hwasan/check-interface.c: New test. * c-c++-common/hwasan/halt_on_error-1.c: New test. * c-c++-common/hwasan/handles-poly_int-marked-vars.c: New test. * c-c++-common/hwasan/heap-overflow.c: New test. * c-c++-common/hwasan/hwasan-poison-optimisation.c: New test. * c-c++-common/hwasan/hwasan-thread-access-parent.c: New test. * c-c++-common/hwasan/hwasan-thread-basic-failure.c: New test. * c-c++-common/hwasan/hwasan-thread-clears-stack.c: New test. * c-c++-common/hwasan/hwasan-thread-success.c: New test. * c-c++-common/hwasan/kernel-defaults.c: New test. * c-c++-common/hwasan/large-aligned-0.c: New test. * c-c++-common/hwasan/large-aligned-1.c: New test. * c-c++-common/hwasan/large-aligned-untagging-0.c: New test. * c-c++-common/hwasan/large-aligned-untagging-1.c: New test. * c-c++-common/hwasan/large-aligned-untagging-2.c: New test. * c-c++-common/hwasan/large-aligned-untagging-3.c: New test. * c-c++-common/hwasan/large-aligned-untagging-4.c: New test. * c-c++-common/hwasan/large-aligned-untagging-5.c: New test. * c-c++-common/hwasan/large-aligned-untagging-6.c: New test. * c-c++-common/hwasan/large-aligned-untagging-7.c: New test. * c-c++-common/hwasan/macro-definition.c: New test. * c-c++-common/hwasan/no-sanitize-attribute.c: New test. * c-c++-common/hwasan/param-instrument-mem-intrinsics.c: New test. * c-c++-common/hwasan/param-instrument-reads-and-writes.c: New test. * c-c++-common/hwasan/param-instrument-reads.c: New test. * c-c++-common/hwasan/param-instrument-writes.c: New test. * c-c++-common/hwasan/random-frame-tag.c: New test. * c-c++-common/hwasan/sanity-check-pure-c.c: New test. * c-c++-common/hwasan/setjmp-longjmp-0.c: New test. * c-c++-common/hwasan/setjmp-longjmp-1.c: New test. * c-c++-common/hwasan/stack-tagging-basic-0.c: New test. * c-c++-common/hwasan/stack-tagging-basic-1.c: New test. * c-c++-common/hwasan/stack-tagging-disable.c: New test. * c-c++-common/hwasan/unprotected-allocas-0.c: New test. * c-c++-common/hwasan/unprotected-allocas-1.c: New test. * c-c++-common/hwasan/use-after-free.c: New test. * c-c++-common/hwasan/vararray-outside-caught.c: New test. * c-c++-common/hwasan/vararray-stack-restore-correct.c: New test. * c-c++-common/hwasan/very-large-objects.c: New test. * g++.dg/hwasan/hwasan.exp: New test. * g++.dg/hwasan/rvo-handled.C: New test. * gcc.dg/hwasan/hwasan.exp: New test. * gcc.dg/hwasan/nested-functions-0.c: New test. * gcc.dg/hwasan/nested-functions-1.c: New test. * gcc.dg/hwasan/nested-functions-2.c: New test. * lib/hwasan-dg.exp: New test.
On 13/11/2020 17:22, Martin Liška wrote: > On 11/13/20 5:57 PM, Matthew Malcomson wrote: >> Hi there, >> >> Thanks for the heads-up. >> As it turns out the most recent `libhwasan` crashes when displaying an >> address on the stack in Linux. > > Hello. > > What a bad luck. > >> >> I'm currently working on getting it fixed here >> https://reviews.llvm.org/D91344#2393371 >> <https://reviews.llvm.org/D91344#2393371> . >> If this hwasan patch series gets approved and if that patch goes in >> would it be feasible to bump the libsanitizer merge to whatever >> version that would be? >> >> If not (maybe because stage1 would be finished?) then could/would we >> end up using the LOCAL_PATCHES approach? > > Since now, I would prefer doing cherry picks. Hopefully, we'll end just > with couple of patches. That makes sense, there's just one patch I need 83ac1820. As far as I can tell from the history, the process is simply to apply the patch in GCC, commit it with a ChangeLog etc as if it were a GCC patch, and then add the hash into LOCAL_PATCHES as a separate commit. Is that right? Given that it looks like the hwasan patch series is nearing going in now, I'd like to make sure I know how the library is getting added. Is the plan something like the below? 1) You add the libhwasan update (i.e. this patch you posted). 2) I add the cherry-pick from LLVM compiler-rt (once it's approved) and a separate commit updating LOCAL_PATCHES. 3) I add the hwasan patch series. Or would it make more sense for me to apply your patch below with `--author` using your details (so it goes in the ChangeLog that way)? Thanks! Matthew > > Thanks, > Martin > >> >> Thanks, >> Matthew >> ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------ >> >> *From:* Martin Liška <mliska@suse.cz> >> *Sent:* 13 November 2020 16:33 >> *To:* Matthew Malcomson <Matthew.Malcomson@arm.com>; >> gcc-patches@gcc.gnu.org <gcc-patches@gcc.gnu.org> >> *Cc:* jakub@redhat.com <jakub@redhat.com>; Richard Earnshaw >> <Richard.Earnshaw@arm.com>; kcc@google.com <kcc@google.com>; >> dodji@redhat.com <dodji@redhat.com>; joseph@codesourcery.com >> <joseph@codesourcery.com> >> *Subject:* Re: [Patch 0/X] HWASAN v4 >> On 10/16/20 11:03 AM, Martin Li�ka wrote: >>> Hello. >>> >>> I've just merged libsanitizer and there's the corresponding part that >>> includes >>> libhwasan. >>> >>> Martin >> >> Hey. >> >> I've just made last merge from upstream, there's corresponding hwasan >> part. >> >> Martin >
Matthew Malcomson <matthew.malcomson@arm.com> writes: > Adding hwasan tests. > > Only interesting thing here is that we have to make sure the tagging mechanism > is deterministic to avoid flaky tests. Sorry for not reviewing this one earlier. TBH I only spot-checked the tests themselves (they look good). But on hwasan-dg.exp: I think we should try to avoid so much cut-&-paste between asan-dg.exp and hwasan-dg.exp. For one thing (and obviously not your fault), it seems odd to me that check_effective_target_fsanitize_address is defined in asan-dg.exp. I think it and the new check_effective_targets* should be defined in target-supports.exp instead. On: > +proc check_effective_target_hwaddress_exec {} { > + if ![check_runtime hwaddress_exec { > + int main (void) { return 0; } > + }] { > + return 0; > + } > + return 1; > + > + # hwasan doesn't work if there's a ulimit on virtual memory. > + if ![is_remote target] { > + if [catch {exec sh -c "ulimit -v"} ulimit_v] { > + # failed to get ulimit > + } elseif [regexp {^[0-9]+$} $ulimit_v] { > + # ulimit -v gave a numeric limit > + warning "skipping hwasan tests due to ulimit -v" > + return 0; > + } > + } > +} either the “hwasan doesn't work” block or the early “return 1” should be removed. (I'm guessing the former.) > +proc hwasan_include_flags {} { > + global srcdir > + global TESTING_IN_BUILD_TREE > + > + set flags "" > + > + if { [is_remote host] || ! [info exists TESTING_IN_BUILD_TREE] } { > + return "${flags}" > + } > + > + set flags "-I$srcdir/../../libsanitizer/include" > + > + return "$flags" > +} This is identical to the asan version, but I guess it's small enough that the cut-&-paste doesn't matter. > + > +# > +# hwasan_link_flags -- compute library path and flags to find libhwasan. > +# (originally from g++.exp) > +# > + > +proc hwasan_link_flags { paths } { > + global srcdir > + global ld_library_path > + global shlib_ext > + global hwasan_saved_library_path > + > + set gccpath ${paths} > + set flags "" > + > + set shlib_ext [get_shlib_extension] > + set hwasan_saved_library_path $ld_library_path > + > + if { $gccpath != "" } { > + if { [file exists "${gccpath}/libsanitizer/hwasan/.libs/libhwasan.a"] > + || [file exists "${gccpath}/libsanitizer/hwasan/.libs/libhwasan.${shlib_ext}"] } { > + append flags " -B${gccpath}/libsanitizer/ " > + append flags " -B${gccpath}/libsanitizer/hwasan/ " > + append flags " -L${gccpath}/libsanitizer/hwasan/.libs " > + append ld_library_path ":${gccpath}/libsanitizer/hwasan/.libs" > + } > + } else { > + global tool_root_dir > + > + set libhwasan [lookfor_file ${tool_root_dir} libhwasan] > + if { $libhwasan != "" } { > + append flags "-L${libhwasan} " > + append ld_library_path ":${libhwasan}" > + } > + } > + > + set_ld_library_path_env_vars > + > + return "$flags" > +} Here I'd suggest: - In asan-dg.exp, have: # Compute library path and flags to find libsanitizer library LIB. # (originally from g++.exp). proc asan_link_flags_1 { paths lib } { …body… } …existing comment… proc asan_link_flags { paths } { return [asan_link_flags_1 $paths asan] } where …body… is more or less the current body of asan_link_flags with “asan” replaced by ${lib}. E.g.: global ${lib}_saved_library_path … set ${lib}_saved_library_path $ld_library_path is fine. For local variables like: set libasan [lookfor_file ${tool_root_dir} libasan] if { $libasan != "" } { append flags "-L${libasan} " append ld_library_path ":${libasan}" } it would make more sense to use a generic name instead, e.g.: set libdir [lookfor_file ${tool_root_dir} lib${lib}] if { $libdir != "" } { append flags "-L${libdir} " append ld_library_path ":${libdir}" } - Have hwasan-dg.exp include asan-dg.exp. - Have: proc hwasan_link_flags { paths } { return [asan_link_flags_1 $paths hwasan] } > + > +# > +# hwasan_init -- called at the start of each subdir of tests > +# > + > +proc hwasan_init { args } { > + global TEST_ALWAYS_FLAGS > + global ALWAYS_CXXFLAGS > + global TOOL_OPTIONS > + global hwasan_saved_TEST_ALWAYS_FLAGS > + global hwasan_saved_ALWAYS_CXXFLAGS > + > + setenv HWASAN_OPTIONS "random_tags=0" > + > + set link_flags "" > + if ![is_remote host] { > + if [info exists TOOL_OPTIONS] { > + set link_flags "[hwasan_link_flags [get_multilibs ${TOOL_OPTIONS}]]" > + } else { > + set link_flags "[hwasan_link_flags [get_multilibs]]" > + } > + } > + > + set include_flags "[hwasan_include_flags]" > + > + if [info exists TEST_ALWAYS_FLAGS] { > + set hwasan_saved_TEST_ALWAYS_FLAGS $TEST_ALWAYS_FLAGS > + } > + if [info exists ALWAYS_CXXFLAGS] { > + set hwasan_saved_ALWAYS_CXXFLAGS $ALWAYS_CXXFLAGS > + set ALWAYS_CXXFLAGS [concat "{ldflags=$link_flags}" $ALWAYS_CXXFLAGS] > + set ALWAYS_CXXFLAGS [concat "{additional_flags=-fsanitize=hwaddress --param hwasan-random-frame-tag=0 -g $include_flags}" $ALWAYS_CXXFLAGS] > + } else { > + if [info exists TEST_ALWAYS_FLAGS] { > + set TEST_ALWAYS_FLAGS "$link_flags -fsanitize=hwaddress --param hwasan-random-frame-tag=0 -g $include_flags $TEST_ALWAYS_FLAGS" > + } else { > + set TEST_ALWAYS_FLAGS "$link_flags -fsanitize=hwaddress --param hwasan-random-frame-tag=0 -g $include_flags" > + } > + } > +} I agree this one is different enough from the asan version that it isn't worth commonising the code. > + > +# > +# hwasan_finish -- called at the start of each subdir of tests > +# > + > +proc hwasan_finish { args } { > + global TEST_ALWAYS_FLAGS > + global hwasan_saved_TEST_ALWAYS_FLAGS > + global hwasan_saved_ALWAYS_CXXFLAGS > + global hwasan_saved_library_path > + global ld_library_path > + > + unsetenv HWASAN_OPTIONS > + > + if [info exists hwasan_saved_ALWAYS_CXXFLAGS ] { > + set ALWAYS_CXXFLAGS $hwasan_saved_ALWAYS_CXXFLAGS > + } else { > + if [info exists hwasan_saved_TEST_ALWAYS_FLAGS] { > + set TEST_ALWAYS_FLAGS $hwasan_saved_TEST_ALWAYS_FLAGS > + } else { > + unset TEST_ALWAYS_FLAGS > + } > + } > + set ld_library_path $hwasan_saved_library_path > + set_ld_library_path_env_vars > + clear_effective_target_cache > +} And I guess then it would be more consistent to keep this separate too, if the _init function is. > + > +# Symbolize lines like > +# #2 0xdeadbeef (/some/path/libsanitizer.so.0.0.0+0xbeef) > +# in $output using addr2line to > +# #2 0xdeadbeef in foobar file:123 > +proc hwasan_symbolize { output } { > + set addresses [regexp -inline -all -line "^ *#\[0-9\]+ 0x\[0-9a-f\]+ \[(\](\[^)\]+)\[+\](0x\[0-9a-f\]+)\[)\]$" "$output"] > + if { [llength $addresses] > 0 } { > + set addr2line_name [find_binutils_prog addr2line] > + set idx 1 > + while { $idx < [llength $addresses] } { > + set key [regsub -all "\[\]\[\]" [lindex $addresses $idx] "\\\\&"] > + set val [lindex $addresses [expr $idx + 1]] > + lappend arr($key) $val > + set idx [expr $idx + 3] > + } > + foreach key [array names arr] { > + set args "-f -e $key $arr($key)" > + set status [remote_exec host "$addr2line_name" "$args"] > + if { [lindex $status 0] > 0 } continue > + regsub -all "\r\n" [lindex $status 1] "\n" addr2line_output > + regsub -all "\[\n\r\]BFD: \[^\n\r\]*" $addr2line_output "" addr2line_output > + regsub -all "^BFD: \[^\n\r\]*\[\n\r\]" $addr2line_output "" addr2line_output > + set addr2line_output [regexp -inline -all -line "^\[^\n\r]*" $addr2line_output] > + set idx 0 > + foreach val $arr($key) { > + if { [expr $idx + 1] < [llength $addr2line_output] } { > + set fnname [lindex $addr2line_output $idx] > + set fileline [lindex $addr2line_output [expr $idx + 1]] > + if { "$fnname" != "??" } { > + set newkey "$key+$val" > + set repl($newkey) "$fnname $fileline" > + } > + set idx [expr $idx + 2] > + } > + } > + } > + set idx 0 > + set new_output "" > + while {[regexp -start $idx -indices " #\[0-9\]+ 0x\[0-9a-f\]+ \[(\](\[^)\]+\[+\]0x\[0-9a-f\]+)\[)\]" "$output" -> addr] > 0} { > + set low [lindex $addr 0] > + set high [lindex $addr 1] > + set val [string range "$output" $low $high] > + append new_output [string range "$output" $idx [expr $low - 2]] > + if [info exists repl($val)] { > + append new_output "in $repl($val)" > + } else { > + append new_output "($val)" > + } > + set idx [expr $high + 2] > + } > + append new_output [string range "$output" $idx [string length "$output"]] > + return "$new_output" > + } > + return "$output" > +} > + > +# Return a list of gtest tests, printed in the form > +# DEJAGNU_GTEST_TEST AddressSanitizer_SimpleDeathTest > +# DEJAGNU_GTEST_TEST AddressSanitizer_VariousMallocsTest > +proc hwasan_get_gtest_test_list { output } { > + set idx 0 > + set ret "" > + while {[regexp -start $idx -indices "DEJAGNU_GTEST_TEST (\[^\n\r\]*)(\r\n|\n|\r)" "$output" -> testname] > 0} { > + set low [lindex $testname 0] > + set high [lindex $testname 1] > + set val [string range "$output" $low $high] > + lappend ret $val > + set idx [expr $high + 1] > + } > + return $ret > +} > + > +# Return a list of gtest EXPECT_DEATH tests, printed in the form > +# DEJAGNU_GTEST_EXPECT_DEATH1 statement DEJAGNU_GTEST_EXPECT_DEATH1 regexp DEJAGNU_GTEST_EXPECT_DEATH1 > +# DEJAGNU_GTEST_EXPECT_DEATH2 other statement DEJAGNU_GTEST_EXPECT_DEATH2 other regexp DEJAGNU_GTEST_EXPECT_DEATH2 > +proc hwasan_get_gtest_expect_death_list { output } { > + set idx 0 > + set ret "" > + while {[regexp -start $idx -indices "DEJAGNU_GTEST_EXPECT_DEATH(\[0-9\]*)" "$output" -> id ] > 0} { > + set low [lindex $id 0] > + set high [lindex $id 1] > + set val_id [string range "$output" $low $high] > + if {[regexp -start $low -indices "$val_id (.*) DEJAGNU_GTEST_EXPECT_DEATH$val_id (.*) DEJAGNU_GTEST_EXPECT_DEATH$val_id\[\n\r\]" "$output" whole statement regexpr ] == 0} { break } > + set low [lindex $statement 0] > + set high [lindex $statement 1] > + set val_statement [string range "$output" $low $high] > + set low [lindex $regexpr 0] > + set high [lindex $regexpr 1] > + set val_regexpr [string range "$output" $low $high] > + lappend ret [list "$val_id" "$val_statement" "$val_regexpr"] > + set idx [lindex $whole 1] > + } > + return $ret > +} AFAICT these three functions are identical to the asan-dg.exp versions, with no asan/hwasan variance. I think it would be better to reuse the asan versions for hwasan too. > + > +# Replace ${tool}_load with a wrapper so that we can symbolize the output. > +if { [info procs ${tool}_load] != [list] \ > + && [info procs saved_hwasan_${tool}_load] == [list] } { > + rename ${tool}_load saved_hwasan_${tool}_load > + > + proc ${tool}_load { program args } { > + global tool > + global hwasan_last_gtest_test_list > + global hwasan_last_gtest_expect_death_list > + set result [eval [list saved_hwasan_${tool}_load $program] $args] > + set output [lindex $result 1] > + set symbolized_output [hwasan_symbolize "$output"] > + set hwasan_last_gtest_test_list [hwasan_get_gtest_test_list "$output"] > + set hwasan_last_gtest_expect_death_list [hwasan_get_gtest_expect_death_list "$output"] > + set result [list [lindex $result 0] $symbolized_output] > + return $result > + } > +} Also, I'm not sure we need separate hwasan versions of these globals. > + > +# Utility for running gtest hwasan emulation under dejagnu, invoked via dg-final. > +# Call pass if variable has the desired value, otherwise fail. > +# > +# Argument 0 handles expected failures and the like > +proc hwasan-gtest { args } { > + global tool > + global hwasan_last_gtest_test_list > + global hwasan_last_gtest_expect_death_list > + > + if { ![info exists hwasan_last_gtest_test_list] } { return } > + if { [llength $hwasan_last_gtest_test_list] == 0 } { return } > + if { ![isnative] || [is_remote target] } { return } > + > + set gtest_test_list $hwasan_last_gtest_test_list > + unset hwasan_last_gtest_test_list > + > + if { [llength $args] >= 1 } { > + switch [dg-process-target [lindex $args 0]] { > + "S" { } > + "N" { return } > + "F" { setup_xfail "*-*-*" } > + "P" { } > + } > + } > + > + # This assumes that we are three frames down from dg-test, and that > + # it still stores the filename of the testcase in a local variable "name". > + # A cleaner solution would require a new DejaGnu release. > + upvar 2 name testcase > + upvar 2 prog prog > + > + set output_file "[file rootname [file tail $prog]].exe" > + > + foreach gtest $gtest_test_list { > + set testname "$testcase $gtest" > + set status -1 > + > + setenv DEJAGNU_GTEST_ARG "$gtest" > + set result [${tool}_load ./$output_file $gtest] > + unsetenv DEJAGNU_GTEST_ARG > + set status [lindex $result 0] > + set output [lindex $result 1] > + if { "$status" == "pass" } { > + pass "$testname execution test" > + if { [info exists hwasan_last_gtest_expect_death_list] } { > + set gtest_expect_death_list $hwasan_last_gtest_expect_death_list > + foreach gtest_death $gtest_expect_death_list { > + set id [lindex $gtest_death 0] > + set testname "$testcase $gtest [lindex $gtest_death 1]" > + set regexpr [lindex $gtest_death 2] > + set status -1 > + > + setenv DEJAGNU_GTEST_ARG "$gtest:$id" > + set result [${tool}_load ./$output_file "$gtest:$id"] > + unsetenv DEJAGNU_GTEST_ARG > + set status [lindex $result 0] > + set output [lindex $result 1] > + if { "$status" == "fail" } { > + pass "$testname execution test" > + if { ![regexp $regexpr ${output}] } { > + fail "$testname output pattern test" > + send_log "Output should match: $regexpr\n" > + } else { > + pass "$testname output pattern test" > + } > + } elseif { "$status" == "pass" } { > + fail "$testname execution test" > + } else { > + $status "$testname execution test" > + } > + } > + } > + } else { > + $status "$testname execution test" > + } > + unset hwasan_last_gtest_expect_death_list > + } > + > + return > +} And if there are no separate versions of those variables, and if hwasan-dg.exp does include asan-dg.exp, then I think it would be enough to replace hwasan_symbolize onwards with: # Utility for running gtest hwasan emulation under dejagnu, invoked via dg-final. # Call pass if variable has the desired value, otherwise fail. # # Argument 0 handles expected failures and the like proc hwasan-gtest { args } { asan-gtest {*}$args } Thanks, Richard
On 11/20/20 7:42 PM, Matthew Malcomson wrote: > On 13/11/2020 17:22, Martin Liška wrote: >> On 11/13/20 5:57 PM, Matthew Malcomson wrote: >>> Hi there, >>> >>> Thanks for the heads-up. >>> As it turns out the most recent `libhwasan` crashes when displaying an address on the stack in Linux. >> >> Hello. >> >> What a bad luck. >> >>> >>> I'm currently working on getting it fixed here https://reviews.llvm.org/D91344#2393371 <https://reviews.llvm.org/D91344#2393371> . >>> If this hwasan patch series gets approved and if that patch goes in would it be feasible to bump the libsanitizer merge to whatever version that would be? >>> >>> If not (maybe because stage1 would be finished?) then could/would we end up using the LOCAL_PATCHES approach? >> >> Since now, I would prefer doing cherry picks. Hopefully, we'll end just with couple of patches. > > That makes sense, there's just one patch I need 83ac1820. > > As far as I can tell from the history, the process is simply to apply the patch in GCC, commit it with a ChangeLog etc as if it were a GCC patch, and then add the hash into LOCAL_PATCHES as a separate commit. Hello. Just add it as we did for e.g. b69f33f477b9ac38af3c39465600ae74a3554878. You don't need to write ChangeLog entries. Note that libsanitizer is one of the location where we are not strict about ChangeLog format verification. And yes, then you add it simply to LOCAL_PATCHES. > > Is that right? > > > Given that it looks like the hwasan patch series is nearing going in now, I'd like to make sure I know how the library is getting added. > > Is the plan something like the below? > 1) You add the libhwasan update (i.e. this patch you posted). > 2) I add the cherry-pick from LLVM compiler-rt (once it's approved) > and a separate commit updating LOCAL_PATCHES. > 3) I add the hwasan patch series. Yes. > > Or would it make more sense for me to apply your patch below with `--author` using your details (so it goes in the ChangeLog that way)? Don't worry with that, just apply it as you want. Thanks, Martin > > Thanks! > Matthew > > >> >> Thanks, >> Martin >> >>> >>> Thanks, >>> Matthew >>> ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------ >>> *From:* Martin Liška <mliska@suse.cz> >>> *Sent:* 13 November 2020 16:33 >>> *To:* Matthew Malcomson <Matthew.Malcomson@arm.com>; gcc-patches@gcc.gnu.org <gcc-patches@gcc.gnu.org> >>> *Cc:* jakub@redhat.com <jakub@redhat.com>; Richard Earnshaw <Richard.Earnshaw@arm.com>; kcc@google.com <kcc@google.com>; dodji@redhat.com <dodji@redhat.com>; joseph@codesourcery.com <joseph@codesourcery.com> >>> *Subject:* Re: [Patch 0/X] HWASAN v4 >>> On 10/16/20 11:03 AM, Martin Li�ka wrote: >>>> Hello. >>>> >>>> I've just merged libsanitizer and there's the corresponding part that includes >>>> libhwasan. >>>> >>>> Martin >>> >>> Hey. >>> >>> I've just made last merge from upstream, there's corresponding hwasan part. >>> >>> Martin >> >
Hello, Update of the hwasan tests attached. Ok for trunk? (NOTE on the state of the entire patch series: Currently re-testing everything from scratch to ensure I didn't get tripped up from old state carried around. Also looking into PR 97941 on hwasan. As yet don't understand what's going on there and having difficulty reproducing.) ------------ Adding hwasan tests. Only interesting thing here is that we have to make sure the tagging mechanism is deterministic to avoid flaky tests. gcc/testsuite/ChangeLog: * c-c++-common/ubsan/sanitize-recover-7.c: Update error message format. * lib/asan-dg.exp (check_effective_target_fsanitize_address): Move to target-supports.exp. (asan_link_flags): Implement as a helper function asan_link_flags_1 which asan_link_flags and hwasan_link_flags use. (asan_link_flags_1): Parametrised version of asan_link_flags. * lib/target-supports.exp (check_effective_target_fsanitize_address): Move from asan-dg.exp. (check_effective_target_fsanitize_hwaddress, check_effective_target_hwaddress_exec): New. * c-c++-common/hwasan/aligned-alloc.c: New test. * c-c++-common/hwasan/alloca-array-accessible.c: New test. * c-c++-common/hwasan/alloca-base-init.c: New test. * c-c++-common/hwasan/alloca-gets-different-tag.c: New test. * c-c++-common/hwasan/alloca-outside-caught.c: New test. * c-c++-common/hwasan/arguments-1.c: New test. * c-c++-common/hwasan/arguments-2.c: New test. * c-c++-common/hwasan/arguments-3.c: New test. * c-c++-common/hwasan/arguments.c: New test. * c-c++-common/hwasan/asan-pr63316.c: New test. * c-c++-common/hwasan/asan-pr70541.c: New test. * c-c++-common/hwasan/asan-pr78106.c: New test. * c-c++-common/hwasan/asan-pr79944.c: New test. * c-c++-common/hwasan/asan-rlimit-mmap-test-1.c: New test. * c-c++-common/hwasan/bitfield-1.c: New test. * c-c++-common/hwasan/bitfield-2.c: New test. * c-c++-common/hwasan/builtin-special-handling.c: New test. * c-c++-common/hwasan/check-interface.c: New test. * c-c++-common/hwasan/halt_on_error-1.c: New test. * c-c++-common/hwasan/handles-poly_int-marked-vars.c: New test. * c-c++-common/hwasan/heap-overflow.c: New test. * c-c++-common/hwasan/hwasan-poison-optimisation.c: New test. * c-c++-common/hwasan/hwasan-thread-access-parent.c: New test. * c-c++-common/hwasan/hwasan-thread-basic-failure.c: New test. * c-c++-common/hwasan/hwasan-thread-clears-stack.c: New test. * c-c++-common/hwasan/hwasan-thread-success.c: New test. * c-c++-common/hwasan/kernel-defaults.c: New test. * c-c++-common/hwasan/large-aligned-0.c: New test. * c-c++-common/hwasan/large-aligned-1.c: New test. * c-c++-common/hwasan/large-aligned-untagging-0.c: New test. * c-c++-common/hwasan/large-aligned-untagging-1.c: New test. * c-c++-common/hwasan/large-aligned-untagging-2.c: New test. * c-c++-common/hwasan/large-aligned-untagging-3.c: New test. * c-c++-common/hwasan/large-aligned-untagging-4.c: New test. * c-c++-common/hwasan/large-aligned-untagging-5.c: New test. * c-c++-common/hwasan/large-aligned-untagging-6.c: New test. * c-c++-common/hwasan/large-aligned-untagging-7.c: New test. * c-c++-common/hwasan/macro-definition.c: New test. * c-c++-common/hwasan/no-sanitize-attribute.c: New test. * c-c++-common/hwasan/param-instrument-mem-intrinsics.c: New test. * c-c++-common/hwasan/param-instrument-reads-and-writes.c: New test. * c-c++-common/hwasan/param-instrument-reads.c: New test. * c-c++-common/hwasan/param-instrument-writes.c: New test. * c-c++-common/hwasan/random-frame-tag.c: New test. * c-c++-common/hwasan/sanity-check-pure-c.c: New test. * c-c++-common/hwasan/setjmp-longjmp-0.c: New test. * c-c++-common/hwasan/setjmp-longjmp-1.c: New test. * c-c++-common/hwasan/stack-tagging-basic-0.c: New test. * c-c++-common/hwasan/stack-tagging-basic-1.c: New test. * c-c++-common/hwasan/stack-tagging-disable.c: New test. * c-c++-common/hwasan/unprotected-allocas-0.c: New test. * c-c++-common/hwasan/unprotected-allocas-1.c: New test. * c-c++-common/hwasan/use-after-free.c: New test. * c-c++-common/hwasan/vararray-outside-caught.c: New test. * c-c++-common/hwasan/vararray-stack-restore-correct.c: New test. * c-c++-common/hwasan/very-large-objects.c: New test. * g++.dg/hwasan/hwasan.exp: New test. * g++.dg/hwasan/rvo-handled.C: New test. * gcc.dg/hwasan/hwasan.exp: New test. * gcc.dg/hwasan/nested-functions-0.c: New test. * gcc.dg/hwasan/nested-functions-1.c: New test. * gcc.dg/hwasan/nested-functions-2.c: New test. * lib/hwasan-dg.exp: New file.
Matthew Malcomson <matthew.malcomson@arm.com> writes: > new file mode 100644 > index 0000000000000000000000000000000000000000..a6a11866823ae8ba9c20b79ac068e84aa73e053d > --- /dev/null > +++ b/gcc/testsuite/lib/hwasan-dg.exp > @@ -0,0 +1,121 @@ > +# Copyright (C) 2012-2019 Free Software Foundation, Inc. 2012-2020 > + > +# This program is free software; you can redistribute it and/or modify > +# it under the terms of the GNU General Public License as published by > +# the Free Software Foundation; either version 3 of the License, or > +# (at your option) any later version. > +# > +# This program is distributed in the hope that it will be useful, > +# but WITHOUT ANY WARRANTY; without even the implied warranty of > +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > +# GNU General Public License for more details. > +# > +# You should have received a copy of the GNU General Public License > +# along with GCC; see the file COPYING3. If not see > +# <http://www.gnu.org/licenses/>. > + > +# Return 1 if compilation with -fsanitize=hwaddress is error-free for trivial > +# code, 0 otherwise. > + > +load_lib asan-dg.exp The “Return 1” comment is now misplaced. However… > diff --git a/gcc/testsuite/lib/target-supports.exp b/gcc/testsuite/lib/target-supports.exp > index ceee78c26a9379f908659ae3d594bca104227d04..18b8c5a60a089fcac7bf2a41adcc373f995fca0f 100644 > --- a/gcc/testsuite/lib/target-supports.exp > +++ b/gcc/testsuite/lib/target-supports.exp > @@ -10633,6 +10633,30 @@ proc check_effective_target_movdir { } { > } "-mmovdiri -mmovdir64b" ] > } > > +# Return 1 if compilation with -fsanitize=address is error-free for trivial > +# code, 0 otherwise. > + > +proc check_effective_target_fsanitize_address {} { > + if ![check_no_compiler_messages fsanitize_address executable { > + int main (void) { return 0; } > + }] { > + return 0; > + } > + > + # asan doesn't work if there's a ulimit on virtual memory. > + if ![is_remote target] { > + if [catch {exec sh -c "ulimit -v"} ulimit_v] { > + # failed to get ulimit > + } elseif [regexp {^[0-9]+$} $ulimit_v] { > + # ulimit -v gave a numeric limit > + warning "skipping asan tests due to ulimit -v" > + return 0; > + } > + } > + > + return 1; > +} > + Looking at this again, I realise I was wrong to say that the effective_target routines should be here. I guess this function was in asan-dg.exp because it doesn't add any flags to ensure that the address sanitiser is used. Instead it relies on the harness to add the flags first. So could you put this back but add: # # NOTE: This function should only be used between calls to asan_init # and asan_finish. It is therefore defined here rather than in # target-supports.exp. Similarly for the hwasan routines, but with hwasan_init and hwasan_finish. Sorry for the runaround. OK with that change, no need for another review round. Thanks, Richard
diff --git a/gcc/internal-fn.h b/gcc/internal-fn.h index a1bc081..d81eb12 100644 --- a/gcc/internal-fn.h +++ b/gcc/internal-fn.h @@ -101,10 +101,16 @@ extern void init_internal_fns (); extern const char *const internal_fn_name_array[]; + +extern bool hwasan_sanitize_p (void); static inline const char * internal_fn_name (enum internal_fn fn) { - return internal_fn_name_array[(int) fn]; + const char *ret = internal_fn_name_array[(int) fn]; + if (! strcmp (ret, "ASAN_MARK") + && hwasan_sanitize_p ()) + return "HWASAN_MARK"; + return ret; } extern internal_fn lookup_internal_fn (const char *);