On 11/20/19 6:40 PM, Matthew Malcomson wrote:
On 20/11/2019 14:29, Martin Liška wrote:
On 11/7/19 7:37 PM, Matthew Malcomson wrote:
I have rebased this series onto Martin Liska's patches that take the most
recent libhwasan from upstream LLVM.
https://gcc.gnu.org/ml/gcc-patches/2019-11/msg00340.html
I've also cleared up some nomenclature (I had previously used the word
'colour'
a few times instead of the word 'tag' and that clashes with other
descriptions)
and based the patch series off a more recent GCC revision (r277678).
There's an ongoing discussion on whether to have
__SANITIZER_ADDRESS__, or
__SANITIZER_HWADDRESS__, but I'm keeping that discussion to the existing
thread.
Similarly there's still the question around C++ exceptions that I'm
keeping to
the existing thread (on the first patch series).
NOTE:
Unfortunately, there's a bug in the more recent version of GCC I
rebased
onto.
Hwasan catches this when bootstrapping, which means bootstrapping
with hwasan
fails.
I'm working on tracking the bug down now, but sending this series
upstream
for visibility while that happens.
Bugzilla link:
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=92410
Entire patch series attached to cover letter.=
Hello.
Thank you very much for the patch set, I've just wrote some inline replies
and I have also some questions/suggestions that I'll summarize here. One
caveat,
I'm not maintainer in any of the ideas and I must confirm that I haven't
made
a deep review of the part which relates to new RTL hooks and stack
expansion.
I expect Jakub can help us here.
Questions/notes:
a) For ASAN we do have bunch of parameters:
gcc --help=param | grep asan
asan-stack Enable asan stack protection.
asan-instrument-allocas Enable asan allocas/VLAs protection.
asan-globals Enable asan globals protection.
asan-instrument-writes Enable asan store operations protection.
asan-instrument-reads Enable asan load operations protection.
asan-memintrin Enable asan builtin functions protection.
asan-use-after-return Enable asan detection of use-after-return
bugs.
asan-instrumentation-with-call-threshold Use callbacks instead of
inline code if number of accesses in function becomes greater or equal
to this number.
We can probably use some of these in order to drive hwaddress in a
similar way. Most of them makes sense for hwaddress as well
I would prefer to keep the options different but would be happy to share
them if others want.
Works for me.
I figure that there will be some parameters that make sense for hwasan
but not for asan.
For example hwasan-random-frame-tag doesn't have any analogue for asan.
Re-using `asan-stack` for hwasan would mean we would then need to decide
between having options with either `hwasan` or `asan` prefix being able
to affect hwasan, or introducing a parameter `asan-random-frame-tag`
that has no meaning on asan.
Then, I would come up with additional hwasan-* parameters that can have
the same semantic as the for asan (if it makes sense).
b) Apparently clang prints also value of all registers. Do we want to do
the same?
I believe this only happens on clang for inline checking (try testing
clang using the parameter `-mllvm --hwasan-instrument-with-calls` to see
without).
Are we talking about usage of __hwasan_check_x0_18?
This would be nice to have, but I'm not planning on looking at it any
time soon.
This is currently implemented in clang on top of two not-yet implemented
features: Inline instrumentation, and generated checking functions with
special calling conventions.
It's certainly not something I'm aiming to get into GCC 10.
Which is fine.
c) I'm a bit confused by the pointer tags, where clang does:
Yeah -- I left out a description of short-tags.
Hopefully my explanation in the below email helped.
https://gcc.gnu.org/ml/gcc-patches/2019-11/msg01968.html
d) Are you planning to come up with inline stack variable
tagging/untagging for GCC 10?
To make sure I understand the question correctly:
I think you're asking about writing tags into the shadow memory.
Yes, which will be very similar to current ASAN code in
asan_emit_stack_protection.
I would expect a significant speed up from the inline shadow memory tag
emission?
I wasn't planning on this.
What I've posted has all the functionality I'm aiming to get in.
My stretch goal at the moment is to handle exceptions (where I currently
have a fundamental problem I'm trying to resolve).
e) In hwasan_increment_tag, shouldn't you skip HWASAN_STACK_BACKGROUND
color?
Hopefully answered in
https://gcc.gnu.org/ml/gcc-patches/2019-11/msg01968.html
Yes.
f) I would rename ASAN_MARK in tree dumps to HWASAN_MARK, similarly to
what you did
for HWASAN_CHECK?
This is an artifact of a shortcut I made (and tried but probably failed
to explain well in
https://gcc.gnu.org/ml/gcc-patches/2019-09/msg00392.html).
For HWASAN_CHECK I introduced a new internal function that takes the
same arguments as ASAN_CHECK, hence this new function is printed
differently.
For HWASAN_MARK, I used a trick to avoid adding "almost duplicate" code
everywhere in the mid-end that checked for ASAN_MARK.
Then, in the sanopt pass (i.e. later on in compilation) I turn that
ASAN_MARK internal function into a HWASAN_MARK internal function.
This happens after all passes that check for ASAN_MARK so it should be a
problem .
I know that you are using ASAN_MARK in -fsanitize=hwaddress. What I said
is maybe we want to print in dump files {HW,.}ASAN_MARK based on what
sanitizer is used (as one can't utilize both at the same time). But it's
a nit.
Otherwise, everywhere that a function handles the case of coming across
ASAN_MARK, I would need to account for HWASAN_MARK by doing essentially
the same thing and I thought that would be a bit of a pain.
I would be happy to use the more explicit way of having two seperate
functions -- not certain which I'd prefer myself.
g) I noticed quite some GNU coding style violations: 'if (! my_cond)',
should be 'if (!my_cond)',
but it's a nit.
Will take a look.
h) I maybe found a bug for:
Hopefully answered in
https://gcc.gnu.org/ml/gcc-patches/2019-11/msg01968.html
i) As mentioned, we would appreciate a comment before each newly added
function :)
Thanks,
Martin
Will do ;-)
As said, thanks for working on that.
Martin
