Page MenuHomePhabricator

[asan] debug_mapping.cc should also pass when the leading digit is hexadecimal.
ClosedPublic

Authored by dsanders on Apr 22 2015, 9:22 AM.

Details

Summary

Previously the CHECK directives only matched decimal digits causing it to match
'7' on x86_64 instead of the whole value.

This fixes a failure on mips-linux-gnu targets where the leading digit is 'a'.

Diff Detail

Event Timeline

dsanders updated this revision to Diff 24235.Apr 22 2015, 9:22 AM
dsanders retitled this revision from to [asan] debug_mapping.cc should also pass when the leading digit is hexadecimal..
dsanders updated this object.
dsanders edited the test plan for this revision. (Show Details)
dsanders added reviewers: kcc, timurrrr, slthakur.
dsanders added a subscriber: Unknown Object (MLST).
timurrrr edited edge metadata.Apr 22 2015, 10:09 AM

Can you please provide an example output that doesn't match this?

ср, 22 апр. 2015, 19:23, Daniel Sanders <daniel.sanders@imgtec.com>:

Hi kcc, timurrrr, sagar,

Previously the CHECK directives only matched decimal digits causing it to
match
'7' on x86_64 instead of the whole value.

This fixes a failure on mips-linux-gnu targets where the leading digit is
'a'.

http://reviews.llvm.org/D9199

Files:

test/asan/TestCases/debug_mapping.cc

Index: test/asan/TestCases/debug_mapping.cc
===================================================================

  • test/asan/TestCases/debug_mapping.cc +++ test/asan/TestCases/debug_mapping.cc @@ -7,8 +7,8 @@ #include <stdlib.h>

    printed because of verbosity=1 - CHECK: SHADOW_SCALE: [[SCALE:[0-9]+]] - CHECK: SHADOW_OFFSET: [[OFFSET:[0-9]+]] + CHECK: SHADOW_SCALE: [[SCALE:[0-9a-f]+]] +// CHECK: SHADOW_OFFSET: [[OFFSET:[0-9a-f]+]]

    int main() { size_t scale, offset;

    EMAIL PREFERENCES http://reviews.llvm.org/settings/panel/emailpreferences/

Sure. On a mips-linux-gnu target I get:
$ env ASAN_OPTIONS=verbosity=1 /home/das/llvm/build/projects/compiler-rt/test/asan/MIPSLinuxConfig/TestCases/Output/debug_mapping.cc.tmp 2>&1

941==AddressSanitizer: failed to intercept '__isoc99_printf'

941==AddressSanitizer: failed to intercept '__isoc99_sprintf'

941==AddressSanitizer: failed to intercept '__isoc99_snprintf'

941==AddressSanitizer: failed to intercept '__isoc99_fprintf'

941==AddressSanitizer: failed to intercept '__isoc99_vprintf'

941==AddressSanitizer: failed to intercept '__isoc99_vsprintf'

941==AddressSanitizer: failed to intercept '__isoc99_vsnprintf'

941==AddressSanitizer: failed to intercept '__isoc99_vfprintf'

941==AddressSanitizer: libc interceptors initialized

[0x2aaa0000, 0xffffffff]HighMem
[0x0fff4000, 0x2aa9ffff]HighShadow
[0x0bff4000, 0x0fff3fff]ShadowGap
[0x0aaa0000, 0x0bff3fff]LowShadow
[0x00000000, 0x0aa9ffff]LowMem

MemToShadow(shadow): 0x0bff4000 0x0c29e7ff 0x0ca9e800 0x0fff3fff
redzone=16
max_redzone=2048
quarantine_size_mb=64M
malloc_context_size=30
SHADOW_SCALE: 3
SHADOW_GRANULARITY: 8
SHADOW_OFFSET: aaa0000

941==Installed the sigaction for signal 11

941==Installed the sigaction for signal 10

941==T0: stack [0x7f43d000,0x7fc3d000) size 0x800000; local=0x7fc3cab8

941==AddressSanitizer Init done

scale: 3
offset: aaa0000

The 'CHECK: SHADOW_OFFSET: [[OFFSET:[0-9]+]]' fails to match 'SHADOW_OFFSET: aaa0000'.

SHADOW_OFFSET expression change -- LGTM

test/asan/TestCases/debug_mapping.cc
10

Please leave the SCALE regexp unchanged

timurrrr accepted this revision.Apr 23 2015, 5:43 AM
timurrrr edited edge metadata.
This revision is now accepted and ready to land.Apr 23 2015, 5:43 AM

I don't mind only changing the SHADOW_OFFSET expression since that fixes my test failure but I have to ask:
Does it really make sense to 'Printf("SHADOW_SCALE: %zx\n", (uptr)SHADOW_SCALE)' in asan_rtl.cc and expect the output to be decimal in the debug_mapping.cc test?

dsanders closed this revision.Apr 23 2015, 6:26 AM

Nice catch, I've overlooked that code!

SHADOW_SCALE is currently a constant equal to 3 and it can't be more than 8 with the current ASan design, I think?
Probably, the same applies to SHADOW_GRANULARITY - it is currently equal to 8, can't be more than 256 and I'd argue we should always print them out as decimals for readability.

That's being said, I think the Printf line you're referring to should be changed to just use %d.
[I suspect this is a side effect of copy-pasting code]

Thanks for working on this!

чт, 23 апр. 2015 г. в 16:54, Daniel Sanders <Daniel.Sanders@imgtec.com>:

> -----Original Message-----
> From: Timur Iskhodzhanov [mailto:timurrrr@google.com]
> Sent: 23 April 2015 14:38
> To: Daniel Sanders; kcc@google.com; Sagar Thakur; timurrrr@google.com
> Cc: llvm-commits@cs.uiuc.edu
> Subject: Re: [PATCH] [asan] debug_mapping.cc should also pass when the
> leading digit is hexadecimal.
>
> Nice catch, I've overlooked that code!
>
> SHADOW_SCALE is currently a constant equal to 3 and it can't be more
> than 8 with the current ASan design, I think?

Now that you mention it, I see there's a 'CHECK(SHADOW_SCALE >= 3 &&
SHADOW_SCALE <= 7);' a few lines down.

> Probably, the same applies to SHADOW_GRANULARITY - it is currently
equal
> to 8, can't be more than 256 and I'd argue we should always print them
out as
> decimals for readability.
>
> That's being said, I think the Printf line you're referring to should
be
> changed to just use %d.
> [I suspect this is a side effect of copy-pasting code]

That makes sense to me. I'll upload a patch for those two shortly.

> http://reviews.llvm.org/D9199
>
> EMAIL PREFERENCES
> http://reviews.llvm.org/settings/panel/emailpreferences/
>


llvm-commits mailing list
llvm-commits@cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits