Re: GetPreviousInstructionPc question

2017-04-20 Thread 'Dmitry Vyukov' via address-sanitizer
On Thu, Apr 20, 2017 at 12:00 PM, Yuri Gribov  wrote:
> On Thu, Apr 20, 2017 at 10:53 AM, 'Dmitry Vyukov' via
> address-sanitizer  wrote:
>> On Thu, Apr 20, 2017 at 11:50 AM, Yuri Gribov  wrote:
>>> On Thu, Apr 20, 2017 at 10:20 AM, 'Dmitry Vyukov' via
>>> address-sanitizer  wrote:
 On Thu, Apr 20, 2017 at 11:11 AM, evgeny777  
 wrote:
> Thanks for clarifying it, Dmitry.
>
> Here is piece of report I get:
>
> ==18244==ERROR: AddressSanitizer: heap-buffer-overflow on address
> 0x6020001a at pc 0x005a9cad bp 0x7ffc10528760 sp 0x7ffc10528740
> WRITE of size 1 at 0x6020001a thread T0
> #0 0x5a9cac  (/home/evgeny/work/linker_scripts/asan/asan+0x5a9cac)
> #1 0x7f310488082f  (/lib/x86_64-linux-gnu/libc.so.6+0x2082f)
> #2 0x419498  (/home/evgeny/work/linker_scripts/asan/asan+0x419498)
>
> 
>
> Below is the piece of disassembly of main :
>
> .
> 0x5a9ca8 <+136>: callq  0x56d9d0  ;
> ::__asan_report_store1(__sanitizer::uptr) at asan_rtl.cc:136
> 0x5a9cad <+141>: xorl   %eax, %eax
> .
>
> As you may noticed 0x5a9cac == (0x5a9cad - 1)


 I think tsan prints unmodified PC and we should do the same in asan.
 This also reliefs us from figuring out correct instruction length on
 ARM/thumb/etc as nobody sees the modified PC.
>>>
>>> Hm, the unmodified PC will make symbolized stacktraces less readable.
>>> What's the problem with "-1"? Addr2line and other bintools work fine
>>> with it.
>>
>> I literally mean "print unmodified PC" (as a hex value). I am not
>> proposing to change how symbolization works.
>
> My understanding is that symbolization code symbolizes "pc-1" as well.
> If we keep symbolization code unchanged and only change printed hex
> value, this will probly cause offline symbolizer to behave differently
> from internal symbolizer.


Okay, I don't think this is worth any more of our time.
If there is a spot improvement to GetPreviousInstructionPc for arm,
that will be fine to do.



> Actually the situation is even more interesting: on ARM we do pc &
> (~1) (not pc-1) to cancel out Thumb bit.
>
> On Thursday, April 20, 2017 at 12:01:25 PM UTC+3, Dmitry Vyukov wrote:
>>
>> On Thu, Apr 20, 2017 at 10:44 AM, evgeny777  wrote:
>> > I noticed that GetPreviousInstructionPc() function returns 'pc - 1' for
>> > both
>> > arm32 and arm64.
>> > This causes odd addresses to appear in stack traces, which is nonsense,
>> > as
>> > both arm32/64 instructions
>> > have 4 byte size and alignment.
>> >
>> > The x86 and x86_64 cases are even more confusing, because instruction
>> > length
>> > is not constant. What exactly this 'pc - 1' is expected to return?
>> >
>> > But even if one is able to get previous instruction address correctly 
>> > he
>> > may
>> > still get confusing results. In case some instruction triggers
>> > hardware exception, its address will go to ASAN stack trace (via
>> > SlowUnwindStackWithContext). Returning address of previous instruction
>> > in such case can be extremely confusing.
>> >
>> > Is there any point in using this function?
>>
>> Hi,
>>
>> Yes, there is a very bold point in using this function.
>> Typically top frame PC is obtained with __builtin_return_address,
>> which means that it points to the next instruction after the call. And
>> we need to obtain debug info associated with the call instruction. To
>> achieve that we subtract 1 from PC. All symbolization code that we've
>> seen is fine with PC pointing into a middle of an instruction.
>>
>> Now, if we print pc-1 in reports (do we?), then it's a bug. We need to
>> print unaltered PC in reports.
>>
>> Re hardware exceptions. This needs to be fixed. A trivial change would
>> be to add 1 to PCs pointing to faulting instruction. Then
>> GetPreviousInstructionPc will offset this and we get correct debug
>> info. However, then we will print incorrect PC in report. So a proper
>> fix would be to augment all stack traces with a flag saying if top PC
>> needs to be adjusted during symbolization or not.
>
> --
> You received this message because you are subscribed to the Google Groups
> "address-sanitizer" group.
> To unsubscribe from this group and stop receiving emails from it, send an
> email to address-sanitizer+unsubscr...@googlegroups.com.
> For more options, visit https://groups.google.com/d/optout.

 --
 You received this message because you are subscribed to the Google Groups 
 "address-sanitizer" group.
 To unsubscribe from this group and stop receiving emails from it, send an 

Re: GetPreviousInstructionPc question

2017-04-20 Thread Eugene Leviant
Yuri, I think on ARM we also subtract 1 from pc:

#if defined(__arm__)
  // Cancel Thumb bit.
  pc = pc & (~1);
#endif
#if defined(__powerpc__) || defined(__powerpc64__)
// ...
#elif defined(__sparc__) || defined(__mips__)
// ...
#else
  return pc - 1; // Called for ARM as well as for Thumb and aarch64
#endif

2017-04-20 13:00 GMT+03:00 Yuri Gribov :
> On Thu, Apr 20, 2017 at 10:53 AM, 'Dmitry Vyukov' via
> address-sanitizer  wrote:
>> On Thu, Apr 20, 2017 at 11:50 AM, Yuri Gribov  wrote:
>>> On Thu, Apr 20, 2017 at 10:20 AM, 'Dmitry Vyukov' via
>>> address-sanitizer  wrote:
 On Thu, Apr 20, 2017 at 11:11 AM, evgeny777  
 wrote:
> Thanks for clarifying it, Dmitry.
>
> Here is piece of report I get:
>
> ==18244==ERROR: AddressSanitizer: heap-buffer-overflow on address
> 0x6020001a at pc 0x005a9cad bp 0x7ffc10528760 sp 0x7ffc10528740
> WRITE of size 1 at 0x6020001a thread T0
> #0 0x5a9cac  (/home/evgeny/work/linker_scripts/asan/asan+0x5a9cac)
> #1 0x7f310488082f  (/lib/x86_64-linux-gnu/libc.so.6+0x2082f)
> #2 0x419498  (/home/evgeny/work/linker_scripts/asan/asan+0x419498)
>
> 
>
> Below is the piece of disassembly of main :
>
> .
> 0x5a9ca8 <+136>: callq  0x56d9d0  ;
> ::__asan_report_store1(__sanitizer::uptr) at asan_rtl.cc:136
> 0x5a9cad <+141>: xorl   %eax, %eax
> .
>
> As you may noticed 0x5a9cac == (0x5a9cad - 1)


 I think tsan prints unmodified PC and we should do the same in asan.
 This also reliefs us from figuring out correct instruction length on
 ARM/thumb/etc as nobody sees the modified PC.
>>>
>>> Hm, the unmodified PC will make symbolized stacktraces less readable.
>>> What's the problem with "-1"? Addr2line and other bintools work fine
>>> with it.
>>
>> I literally mean "print unmodified PC" (as a hex value). I am not
>> proposing to change how symbolization works.
>
> My understanding is that symbolization code symbolizes "pc-1" as well.
> If we keep symbolization code unchanged and only change printed hex
> value, this will probly cause offline symbolizer to behave differently
> from internal symbolizer.
>
> Actually the situation is even more interesting: on ARM we do pc &
> (~1) (not pc-1) to cancel out Thumb bit.
>
> On Thursday, April 20, 2017 at 12:01:25 PM UTC+3, Dmitry Vyukov wrote:
>>
>> On Thu, Apr 20, 2017 at 10:44 AM, evgeny777  wrote:
>> > I noticed that GetPreviousInstructionPc() function returns 'pc - 1' for
>> > both
>> > arm32 and arm64.
>> > This causes odd addresses to appear in stack traces, which is nonsense,
>> > as
>> > both arm32/64 instructions
>> > have 4 byte size and alignment.
>> >
>> > The x86 and x86_64 cases are even more confusing, because instruction
>> > length
>> > is not constant. What exactly this 'pc - 1' is expected to return?
>> >
>> > But even if one is able to get previous instruction address correctly 
>> > he
>> > may
>> > still get confusing results. In case some instruction triggers
>> > hardware exception, its address will go to ASAN stack trace (via
>> > SlowUnwindStackWithContext). Returning address of previous instruction
>> > in such case can be extremely confusing.
>> >
>> > Is there any point in using this function?
>>
>> Hi,
>>
>> Yes, there is a very bold point in using this function.
>> Typically top frame PC is obtained with __builtin_return_address,
>> which means that it points to the next instruction after the call. And
>> we need to obtain debug info associated with the call instruction. To
>> achieve that we subtract 1 from PC. All symbolization code that we've
>> seen is fine with PC pointing into a middle of an instruction.
>>
>> Now, if we print pc-1 in reports (do we?), then it's a bug. We need to
>> print unaltered PC in reports.
>>
>> Re hardware exceptions. This needs to be fixed. A trivial change would
>> be to add 1 to PCs pointing to faulting instruction. Then
>> GetPreviousInstructionPc will offset this and we get correct debug
>> info. However, then we will print incorrect PC in report. So a proper
>> fix would be to augment all stack traces with a flag saying if top PC
>> needs to be adjusted during symbolization or not.
>
> --
> You received this message because you are subscribed to the Google Groups
> "address-sanitizer" group.
> To unsubscribe from this group and stop receiving emails from it, send an
> email to address-sanitizer+unsubscr...@googlegroups.com.
> For more options, visit https://groups.google.com/d/optout.

 --
 You received this message because you 

Re: GetPreviousInstructionPc question

2017-04-20 Thread Yuri Gribov
On Thu, Apr 20, 2017 at 10:53 AM, 'Dmitry Vyukov' via
address-sanitizer  wrote:
> On Thu, Apr 20, 2017 at 11:50 AM, Yuri Gribov  wrote:
>> On Thu, Apr 20, 2017 at 10:20 AM, 'Dmitry Vyukov' via
>> address-sanitizer  wrote:
>>> On Thu, Apr 20, 2017 at 11:11 AM, evgeny777  
>>> wrote:
 Thanks for clarifying it, Dmitry.

 Here is piece of report I get:

 ==18244==ERROR: AddressSanitizer: heap-buffer-overflow on address
 0x6020001a at pc 0x005a9cad bp 0x7ffc10528760 sp 0x7ffc10528740
 WRITE of size 1 at 0x6020001a thread T0
 #0 0x5a9cac  (/home/evgeny/work/linker_scripts/asan/asan+0x5a9cac)
 #1 0x7f310488082f  (/lib/x86_64-linux-gnu/libc.so.6+0x2082f)
 #2 0x419498  (/home/evgeny/work/linker_scripts/asan/asan+0x419498)

 

 Below is the piece of disassembly of main :

 .
 0x5a9ca8 <+136>: callq  0x56d9d0  ;
 ::__asan_report_store1(__sanitizer::uptr) at asan_rtl.cc:136
 0x5a9cad <+141>: xorl   %eax, %eax
 .

 As you may noticed 0x5a9cac == (0x5a9cad - 1)
>>>
>>>
>>> I think tsan prints unmodified PC and we should do the same in asan.
>>> This also reliefs us from figuring out correct instruction length on
>>> ARM/thumb/etc as nobody sees the modified PC.
>>
>> Hm, the unmodified PC will make symbolized stacktraces less readable.
>> What's the problem with "-1"? Addr2line and other bintools work fine
>> with it.
>
> I literally mean "print unmodified PC" (as a hex value). I am not
> proposing to change how symbolization works.

My understanding is that symbolization code symbolizes "pc-1" as well.
If we keep symbolization code unchanged and only change printed hex
value, this will probly cause offline symbolizer to behave differently
from internal symbolizer.

Actually the situation is even more interesting: on ARM we do pc &
(~1) (not pc-1) to cancel out Thumb bit.

 On Thursday, April 20, 2017 at 12:01:25 PM UTC+3, Dmitry Vyukov wrote:
>
> On Thu, Apr 20, 2017 at 10:44 AM, evgeny777  wrote:
> > I noticed that GetPreviousInstructionPc() function returns 'pc - 1' for
> > both
> > arm32 and arm64.
> > This causes odd addresses to appear in stack traces, which is nonsense,
> > as
> > both arm32/64 instructions
> > have 4 byte size and alignment.
> >
> > The x86 and x86_64 cases are even more confusing, because instruction
> > length
> > is not constant. What exactly this 'pc - 1' is expected to return?
> >
> > But even if one is able to get previous instruction address correctly he
> > may
> > still get confusing results. In case some instruction triggers
> > hardware exception, its address will go to ASAN stack trace (via
> > SlowUnwindStackWithContext). Returning address of previous instruction
> > in such case can be extremely confusing.
> >
> > Is there any point in using this function?
>
> Hi,
>
> Yes, there is a very bold point in using this function.
> Typically top frame PC is obtained with __builtin_return_address,
> which means that it points to the next instruction after the call. And
> we need to obtain debug info associated with the call instruction. To
> achieve that we subtract 1 from PC. All symbolization code that we've
> seen is fine with PC pointing into a middle of an instruction.
>
> Now, if we print pc-1 in reports (do we?), then it's a bug. We need to
> print unaltered PC in reports.
>
> Re hardware exceptions. This needs to be fixed. A trivial change would
> be to add 1 to PCs pointing to faulting instruction. Then
> GetPreviousInstructionPc will offset this and we get correct debug
> info. However, then we will print incorrect PC in report. So a proper
> fix would be to augment all stack traces with a flag saying if top PC
> needs to be adjusted during symbolization or not.

 --
 You received this message because you are subscribed to the Google Groups
 "address-sanitizer" group.
 To unsubscribe from this group and stop receiving emails from it, send an
 email to address-sanitizer+unsubscr...@googlegroups.com.
 For more options, visit https://groups.google.com/d/optout.
>>>
>>> --
>>> You received this message because you are subscribed to the Google Groups 
>>> "address-sanitizer" group.
>>> To unsubscribe from this group and stop receiving emails from it, send an 
>>> email to address-sanitizer+unsubscr...@googlegroups.com.
>>> For more options, visit https://groups.google.com/d/optout.
>>
>> --
>> You received this message because you are subscribed to the Google Groups 
>> "address-sanitizer" group.
>> To unsubscribe from this group and stop receiving emails from it, send an 
>> email to 

Re: GetPreviousInstructionPc question

2017-04-20 Thread 'Dmitry Vyukov' via address-sanitizer
On Thu, Apr 20, 2017 at 11:50 AM, Yuri Gribov  wrote:
> On Thu, Apr 20, 2017 at 10:20 AM, 'Dmitry Vyukov' via
> address-sanitizer  wrote:
>> On Thu, Apr 20, 2017 at 11:11 AM, evgeny777  wrote:
>>> Thanks for clarifying it, Dmitry.
>>>
>>> Here is piece of report I get:
>>>
>>> ==18244==ERROR: AddressSanitizer: heap-buffer-overflow on address
>>> 0x6020001a at pc 0x005a9cad bp 0x7ffc10528760 sp 0x7ffc10528740
>>> WRITE of size 1 at 0x6020001a thread T0
>>> #0 0x5a9cac  (/home/evgeny/work/linker_scripts/asan/asan+0x5a9cac)
>>> #1 0x7f310488082f  (/lib/x86_64-linux-gnu/libc.so.6+0x2082f)
>>> #2 0x419498  (/home/evgeny/work/linker_scripts/asan/asan+0x419498)
>>>
>>> 
>>>
>>> Below is the piece of disassembly of main :
>>>
>>> .
>>> 0x5a9ca8 <+136>: callq  0x56d9d0  ;
>>> ::__asan_report_store1(__sanitizer::uptr) at asan_rtl.cc:136
>>> 0x5a9cad <+141>: xorl   %eax, %eax
>>> .
>>>
>>> As you may noticed 0x5a9cac == (0x5a9cad - 1)
>>
>>
>> I think tsan prints unmodified PC and we should do the same in asan.
>> This also reliefs us from figuring out correct instruction length on
>> ARM/thumb/etc as nobody sees the modified PC.
>
> Hm, the unmodified PC will make symbolized stacktraces less readable.
> What's the problem with "-1"? Addr2line and other bintools work fine
> with it.

I literally mean "print unmodified PC" (as a hex value). I am not
proposing to change how symbolization works.

>>> On Thursday, April 20, 2017 at 12:01:25 PM UTC+3, Dmitry Vyukov wrote:

 On Thu, Apr 20, 2017 at 10:44 AM, evgeny777  wrote:
 > I noticed that GetPreviousInstructionPc() function returns 'pc - 1' for
 > both
 > arm32 and arm64.
 > This causes odd addresses to appear in stack traces, which is nonsense,
 > as
 > both arm32/64 instructions
 > have 4 byte size and alignment.
 >
 > The x86 and x86_64 cases are even more confusing, because instruction
 > length
 > is not constant. What exactly this 'pc - 1' is expected to return?
 >
 > But even if one is able to get previous instruction address correctly he
 > may
 > still get confusing results. In case some instruction triggers
 > hardware exception, its address will go to ASAN stack trace (via
 > SlowUnwindStackWithContext). Returning address of previous instruction
 > in such case can be extremely confusing.
 >
 > Is there any point in using this function?

 Hi,

 Yes, there is a very bold point in using this function.
 Typically top frame PC is obtained with __builtin_return_address,
 which means that it points to the next instruction after the call. And
 we need to obtain debug info associated with the call instruction. To
 achieve that we subtract 1 from PC. All symbolization code that we've
 seen is fine with PC pointing into a middle of an instruction.

 Now, if we print pc-1 in reports (do we?), then it's a bug. We need to
 print unaltered PC in reports.

 Re hardware exceptions. This needs to be fixed. A trivial change would
 be to add 1 to PCs pointing to faulting instruction. Then
 GetPreviousInstructionPc will offset this and we get correct debug
 info. However, then we will print incorrect PC in report. So a proper
 fix would be to augment all stack traces with a flag saying if top PC
 needs to be adjusted during symbolization or not.
>>>
>>> --
>>> You received this message because you are subscribed to the Google Groups
>>> "address-sanitizer" group.
>>> To unsubscribe from this group and stop receiving emails from it, send an
>>> email to address-sanitizer+unsubscr...@googlegroups.com.
>>> For more options, visit https://groups.google.com/d/optout.
>>
>> --
>> You received this message because you are subscribed to the Google Groups 
>> "address-sanitizer" group.
>> To unsubscribe from this group and stop receiving emails from it, send an 
>> email to address-sanitizer+unsubscr...@googlegroups.com.
>> For more options, visit https://groups.google.com/d/optout.
>
> --
> You received this message because you are subscribed to the Google Groups 
> "address-sanitizer" group.
> To unsubscribe from this group and stop receiving emails from it, send an 
> email to address-sanitizer+unsubscr...@googlegroups.com.
> For more options, visit https://groups.google.com/d/optout.

-- 
You received this message because you are subscribed to the Google Groups 
"address-sanitizer" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to address-sanitizer+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.


Re: GetPreviousInstructionPc question

2017-04-20 Thread 'Dmitry Vyukov' via address-sanitizer
On Thu, Apr 20, 2017 at 11:11 AM, evgeny777  wrote:
> Thanks for clarifying it, Dmitry.
>
> Here is piece of report I get:
>
> ==18244==ERROR: AddressSanitizer: heap-buffer-overflow on address
> 0x6020001a at pc 0x005a9cad bp 0x7ffc10528760 sp 0x7ffc10528740
> WRITE of size 1 at 0x6020001a thread T0
> #0 0x5a9cac  (/home/evgeny/work/linker_scripts/asan/asan+0x5a9cac)
> #1 0x7f310488082f  (/lib/x86_64-linux-gnu/libc.so.6+0x2082f)
> #2 0x419498  (/home/evgeny/work/linker_scripts/asan/asan+0x419498)
>
> 
>
> Below is the piece of disassembly of main :
>
> .
> 0x5a9ca8 <+136>: callq  0x56d9d0  ;
> ::__asan_report_store1(__sanitizer::uptr) at asan_rtl.cc:136
> 0x5a9cad <+141>: xorl   %eax, %eax
> .
>
> As you may noticed 0x5a9cac == (0x5a9cad - 1)


I think tsan prints unmodified PC and we should do the same in asan.
This also reliefs us from figuring out correct instruction length on
ARM/thumb/etc as nobody sees the modified PC.




> On Thursday, April 20, 2017 at 12:01:25 PM UTC+3, Dmitry Vyukov wrote:
>>
>> On Thu, Apr 20, 2017 at 10:44 AM, evgeny777  wrote:
>> > I noticed that GetPreviousInstructionPc() function returns 'pc - 1' for
>> > both
>> > arm32 and arm64.
>> > This causes odd addresses to appear in stack traces, which is nonsense,
>> > as
>> > both arm32/64 instructions
>> > have 4 byte size and alignment.
>> >
>> > The x86 and x86_64 cases are even more confusing, because instruction
>> > length
>> > is not constant. What exactly this 'pc - 1' is expected to return?
>> >
>> > But even if one is able to get previous instruction address correctly he
>> > may
>> > still get confusing results. In case some instruction triggers
>> > hardware exception, its address will go to ASAN stack trace (via
>> > SlowUnwindStackWithContext). Returning address of previous instruction
>> > in such case can be extremely confusing.
>> >
>> > Is there any point in using this function?
>>
>> Hi,
>>
>> Yes, there is a very bold point in using this function.
>> Typically top frame PC is obtained with __builtin_return_address,
>> which means that it points to the next instruction after the call. And
>> we need to obtain debug info associated with the call instruction. To
>> achieve that we subtract 1 from PC. All symbolization code that we've
>> seen is fine with PC pointing into a middle of an instruction.
>>
>> Now, if we print pc-1 in reports (do we?), then it's a bug. We need to
>> print unaltered PC in reports.
>>
>> Re hardware exceptions. This needs to be fixed. A trivial change would
>> be to add 1 to PCs pointing to faulting instruction. Then
>> GetPreviousInstructionPc will offset this and we get correct debug
>> info. However, then we will print incorrect PC in report. So a proper
>> fix would be to augment all stack traces with a flag saying if top PC
>> needs to be adjusted during symbolization or not.
>
> --
> You received this message because you are subscribed to the Google Groups
> "address-sanitizer" group.
> To unsubscribe from this group and stop receiving emails from it, send an
> email to address-sanitizer+unsubscr...@googlegroups.com.
> For more options, visit https://groups.google.com/d/optout.

-- 
You received this message because you are subscribed to the Google Groups 
"address-sanitizer" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to address-sanitizer+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.


Re: GetPreviousInstructionPc question

2017-04-20 Thread evgeny777
Thanks for clarifying it, Dmitry.

Here is piece of report I get:

==18244==ERROR: AddressSanitizer: heap-buffer-overflow on address 
0x6020001a at pc 0x005a9cad bp 0x7ffc10528760 sp 0x7ffc10528740
WRITE of size 1 at 0x6020001a thread T0
#0 0x5a9cac  (/home/evgeny/work/linker_scripts/asan/asan+0x5a9cac)
#1 0x7f310488082f  (/lib/x86_64-linux-gnu/libc.so.6+0x2082f)
#2 0x419498  (/home/evgeny/work/linker_scripts/asan/asan+0x419498)



Below is the piece of disassembly of main :

. 
0x5a9ca8 <+136>: callq  0x56d9d0  ; 
::__asan_report_store1(__sanitizer::uptr) at asan_rtl.cc:136
0x5a9cad <+141>: xorl   %eax, %eax
.

As you may noticed 0x5a9cac == (0x5a9cad - 1)

On Thursday, April 20, 2017 at 12:01:25 PM UTC+3, Dmitry Vyukov wrote:
>
> On Thu, Apr 20, 2017 at 10:44 AM, evgeny777  > wrote: 
> > I noticed that GetPreviousInstructionPc() function returns 'pc - 1' for 
> both 
> > arm32 and arm64. 
> > This causes odd addresses to appear in stack traces, which is nonsense, 
> as 
> > both arm32/64 instructions 
> > have 4 byte size and alignment. 
> > 
> > The x86 and x86_64 cases are even more confusing, because instruction 
> length 
> > is not constant. What exactly this 'pc - 1' is expected to return? 
> > 
> > But even if one is able to get previous instruction address correctly he 
> may 
> > still get confusing results. In case some instruction triggers 
> > hardware exception, its address will go to ASAN stack trace (via 
> > SlowUnwindStackWithContext). Returning address of previous instruction 
> > in such case can be extremely confusing. 
> > 
> > Is there any point in using this function? 
>
> Hi, 
>
> Yes, there is a very bold point in using this function. 
> Typically top frame PC is obtained with __builtin_return_address, 
> which means that it points to the next instruction after the call. And 
> we need to obtain debug info associated with the call instruction. To 
> achieve that we subtract 1 from PC. All symbolization code that we've 
> seen is fine with PC pointing into a middle of an instruction. 
>
> Now, if we print pc-1 in reports (do we?), then it's a bug. We need to 
> print unaltered PC in reports. 
>
> Re hardware exceptions. This needs to be fixed. A trivial change would 
> be to add 1 to PCs pointing to faulting instruction. Then 
> GetPreviousInstructionPc will offset this and we get correct debug 
> info. However, then we will print incorrect PC in report. So a proper 
> fix would be to augment all stack traces with a flag saying if top PC 
> needs to be adjusted during symbolization or not. 
>

-- 
You received this message because you are subscribed to the Google Groups 
"address-sanitizer" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to address-sanitizer+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.


Re: GetPreviousInstructionPc question

2017-04-20 Thread 'Dmitry Vyukov' via address-sanitizer
On Thu, Apr 20, 2017 at 10:44 AM, evgeny777  wrote:
> I noticed that GetPreviousInstructionPc() function returns 'pc - 1' for both
> arm32 and arm64.
> This causes odd addresses to appear in stack traces, which is nonsense, as
> both arm32/64 instructions
> have 4 byte size and alignment.
>
> The x86 and x86_64 cases are even more confusing, because instruction length
> is not constant. What exactly this 'pc - 1' is expected to return?
>
> But even if one is able to get previous instruction address correctly he may
> still get confusing results. In case some instruction triggers
> hardware exception, its address will go to ASAN stack trace (via
> SlowUnwindStackWithContext). Returning address of previous instruction
> in such case can be extremely confusing.
>
> Is there any point in using this function?

Hi,

Yes, there is a very bold point in using this function.
Typically top frame PC is obtained with __builtin_return_address,
which means that it points to the next instruction after the call. And
we need to obtain debug info associated with the call instruction. To
achieve that we subtract 1 from PC. All symbolization code that we've
seen is fine with PC pointing into a middle of an instruction.

Now, if we print pc-1 in reports (do we?), then it's a bug. We need to
print unaltered PC in reports.

Re hardware exceptions. This needs to be fixed. A trivial change would
be to add 1 to PCs pointing to faulting instruction. Then
GetPreviousInstructionPc will offset this and we get correct debug
info. However, then we will print incorrect PC in report. So a proper
fix would be to augment all stack traces with a flag saying if top PC
needs to be adjusted during symbolization or not.

-- 
You received this message because you are subscribed to the Google Groups 
"address-sanitizer" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to address-sanitizer+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.