Issue 6303097: [Sanitizer] more symbolizer code (Closed)
|
Created:
11 years, 10 months ago by samsonov Modified:
11 years, 10 months ago Base URL:
https://llvm.org/svn/llvm-project/compiler-rt/trunk/lib/ Visibility:
Public. |
DescriptionThis patch extends symbolizer code. In order to symbolize a module, a symbolizer maps it into memory and obtains pointers to necessary debug info sections (last part is implemented on Linux only).
Later these pointers can be passed to constructor of DWARF context-in-memory from LLVM DebugInfo lib.
Patch Set 1 #
Total comments: 10
Patch Set 2 : z #Patch Set 3 : z #
Total comments: 10
Patch Set 4 : z #Patch Set 5 : xz #
MessagesTotal messages: 18
http://codereview.appspot.com/6303097/diff/1/sanitizer_common/sanitizer_common.h File sanitizer_common/sanitizer_common.h (right): http://codereview.appspot.com/6303097/diff/1/sanitizer_common/sanitizer_commo... sanitizer_common/sanitizer_common.h:82: // Debug info routines Looks like a wrong place for it. It's not quite "common". Can it live in symbolizer? http://codereview.appspot.com/6303097/diff/1/sanitizer_common/sanitizer_symbo... File sanitizer_common/sanitizer_symbolizer.cc (right): http://codereview.appspot.com/6303097/diff/1/sanitizer_common/sanitizer_symbo... sanitizer_common/sanitizer_symbolizer.cc:39: DWARFSection debug_info; You don't need them here. You need them only in ctor. http://codereview.appspot.com/6303097/diff/1/sanitizer_common/sanitizer_symbo... sanitizer_common/sanitizer_symbolizer.cc:46: ReadFileToBuffer(module_name, (char**)&addr, &size, kMaxModuleSize); That's a bad idea. You need to use mmap(). Otherwise you will soak up into memory GBs of uninteresting data. http://codereview.appspot.com/6303097/diff/1/sanitizer_common/sanitizer_symbo... sanitizer_common/sanitizer_symbolizer.cc:97: while (proc_maps.Next(&start, &end, &offset, module_name, This ain't gonna work. You need 1 DIContext per *module*. This thing will give you N DIContexts per module (code, code, data, data), and you won't find .bss/.tbss sections at all, because they are no mapped from the file. So you will have N times memory consumption + will be unable to symbolize some data. http://codereview.appspot.com/6303097/diff/1/sanitizer_common/sanitizer_symbo... sanitizer_common/sanitizer_symbolizer.cc:144: module->di_context->getFileLineInfo(module_offset, &info->file, I believe eventually you will want these 2 function calls to be 1 function call. Because llvm's DIContext::SymbolizeCode() is 1 function call.
Sign in to reply to this message.
http://codereview.appspot.com/6303097/diff/1/sanitizer_common/sanitizer_common.h File sanitizer_common/sanitizer_common.h (right): http://codereview.appspot.com/6303097/diff/1/sanitizer_common/sanitizer_commo... sanitizer_common/sanitizer_common.h:82: // Debug info routines On 2012/06/18 18:18:38, dvyukov wrote: > Looks like a wrong place for it. It's not quite "common". Can it live in > symbolizer? Done. http://codereview.appspot.com/6303097/diff/1/sanitizer_common/sanitizer_symbo... File sanitizer_common/sanitizer_symbolizer.cc (right): http://codereview.appspot.com/6303097/diff/1/sanitizer_common/sanitizer_symbo... sanitizer_common/sanitizer_symbolizer.cc:39: DWARFSection debug_info; On 2012/06/18 18:18:38, dvyukov wrote: > You don't need them here. > You need them only in ctor. Done. http://codereview.appspot.com/6303097/diff/1/sanitizer_common/sanitizer_symbo... sanitizer_common/sanitizer_symbolizer.cc:46: ReadFileToBuffer(module_name, (char**)&addr, &size, kMaxModuleSize); On 2012/06/18 18:18:38, dvyukov wrote: > That's a bad idea. You need to use mmap(). Otherwise you will soak up into > memory GBs of uninteresting data. Done. http://codereview.appspot.com/6303097/diff/1/sanitizer_common/sanitizer_symbo... sanitizer_common/sanitizer_symbolizer.cc:97: while (proc_maps.Next(&start, &end, &offset, module_name, On 2012/06/18 18:18:38, dvyukov wrote: > This ain't gonna work. > You need 1 DIContext per *module*. Damn, you're right here. Rewrote this part (use loookup by name to find a debug info context for a module or create one). > This thing will give you N DIContexts per module (code, code, data, data), and > you won't find .bss/.tbss sections at all, because they are no mapped from the > file. So you will have N times memory consumption + will be unable to symbolize > some data. Probably I lack understanding here. OK, what can I do about .bss/.tbss? If they are not mapped into the memory, then how can I get (virtual) address of data in these sections passed to symbolizer? http://codereview.appspot.com/6303097/diff/1/sanitizer_common/sanitizer_symbo... sanitizer_common/sanitizer_symbolizer.cc:144: module->di_context->getFileLineInfo(module_offset, &info->file, On 2012/06/18 18:18:38, dvyukov wrote: > I believe eventually you will want these 2 function calls to be 1 function call. > Because llvm's DIContext::SymbolizeCode() is 1 function call. I don't think returning file/line info and function name in one call to DIContext is the right thing to do (as Benjamin mentioned, these operations are too different in DWARF). Maybe I'll change my mind, though.
Sign in to reply to this message.
On Wed, Jun 20, 2012 at 5:43 AM, <samsonov@google.com> wrote: > Reviewers: dvyukov, kcc1, > > > > http://codereview.appspot.com/**6303097/diff/1/sanitizer_** > common/sanitizer_common.h<http://codereview.appspot.com/6303097/diff/1/sanitizer_common/sanitizer_common.h> > File sanitizer_common/sanitizer_**common.h (right): > > http://codereview.appspot.com/**6303097/diff/1/sanitizer_** > common/sanitizer_common.h#**newcode82<http://codereview.appspot.com/6303097/diff/1/sanitizer_common/sanitizer_common.h#newcode82> > sanitizer_common/sanitizer_**common.h:82: // Debug info routines > On 2012/06/18 18:18:38, dvyukov wrote: > >> Looks like a wrong place for it. It's not quite "common". Can it live >> > in > >> symbolizer? >> > > Done. > > > http://codereview.appspot.com/**6303097/diff/1/sanitizer_** > common/sanitizer_symbolizer.cc<http://codereview.appspot.com/6303097/diff/1/sanitizer_common/sanitizer_symbolizer.cc> > File sanitizer_common/sanitizer_**symbolizer.cc (right): > > http://codereview.appspot.com/**6303097/diff/1/sanitizer_** > common/sanitizer_symbolizer.**cc#newcode39<http://codereview.appspot.com/6303097/diff/1/sanitizer_common/sanitizer_symbolizer.cc#newcode39> > sanitizer_common/sanitizer_**symbolizer.cc:39: DWARFSection debug_info; > On 2012/06/18 18:18:38, dvyukov wrote: > >> You don't need them here. >> You need them only in ctor. >> > > Done. > > > http://codereview.appspot.com/**6303097/diff/1/sanitizer_** > common/sanitizer_symbolizer.**cc#newcode46<http://codereview.appspot.com/6303097/diff/1/sanitizer_common/sanitizer_symbolizer.cc#newcode46> > sanitizer_common/sanitizer_**symbolizer.cc:46: > ReadFileToBuffer(module_name, (char**)&addr, &size, kMaxModuleSize); > On 2012/06/18 18:18:38, dvyukov wrote: > >> That's a bad idea. You need to use mmap(). Otherwise you will soak up >> > into > >> memory GBs of uninteresting data. >> > > Done. > > > http://codereview.appspot.com/**6303097/diff/1/sanitizer_** > common/sanitizer_symbolizer.**cc#newcode97<http://codereview.appspot.com/6303097/diff/1/sanitizer_common/sanitizer_symbolizer.cc#newcode97> > sanitizer_common/sanitizer_**symbolizer.cc:97: while > (proc_maps.Next(&start, &end, &offset, module_name, > On 2012/06/18 18:18:38, dvyukov wrote: > >> This ain't gonna work. >> You need 1 DIContext per *module*. >> > Damn, you're right here. Rewrote this part (use loookup by name to find > a debug info context for a module or create one). > > > This thing will give you N DIContexts per module (code, code, data, >> > data), and > >> you won't find .bss/.tbss sections at all, because they are no mapped >> > from the > >> file. So you will have N times memory consumption + will be unable to >> > symbolize > >> some data. >> > > Probably I lack understanding here. > OK, what can I do about .bss/.tbss? If they are not mapped into the > memory, then how can I get (virtual) address of data in these sections > passed to symbolizer? They are mapped, but not from the file. You can't figure out what it is by looking at /proc/self/maps. It's just an anonymous mapping there. In tsan I use dl_iterate_phdr. It provides all the necessary in information (and w/o text parsing). > > > http://codereview.appspot.com/**6303097/diff/1/sanitizer_** > common/sanitizer_symbolizer.**cc#newcode144<http://codereview.appspot.com/6303097/diff/1/sanitizer_common/sanitizer_symbolizer.cc#newcode144> > sanitizer_common/sanitizer_**symbolizer.cc:144: > module->di_context->**getFileLineInfo(module_offset, &info->file, > On 2012/06/18 18:18:38, dvyukov wrote: > >> I believe eventually you will want these 2 function calls to be 1 >> > function call. > >> Because llvm's DIContext::SymbolizeCode() is 1 function call. >> > > I don't think returning file/line info and function name in one call to > DIContext is the right thing to do (as Benjamin mentioned, these > operations are too different in DWARF). Maybe I'll change my mind, > though. > DIContext has nothing to do with DWARF (well, almost). It's an abstract symbolizer interface. Think what would be a good interface for an abstract symbolizer. Then implement it for DWARF. > > Description: > This patch extends symbolizer code. In order to symbolize a module, a > symbolizer maps it into memory and obtains pointers to necessary debug > info sections (last part is implemented on Linux only). > > Later these pointers can be passed to constructor of DWARF > context-in-memory from LLVM DebugInfo lib. > > > Please review this at http://codereview.appspot.com/**6303097/<http://codereview.appspot.com/6303097/> > > Affected files: > M asan/asan_stack.cc > M sanitizer_common/sanitizer_**common.h > M sanitizer_common/sanitizer_**linux.cc > M sanitizer_common/sanitizer_**mac.cc > M sanitizer_common/sanitizer_**posix.cc > M sanitizer_common/sanitizer_**procmaps.h > M sanitizer_common/sanitizer_**symbolizer.cc > M sanitizer_common/sanitizer_**symbolizer.h > M sanitizer_common/sanitizer_**win.cc > > >
Sign in to reply to this message.
On Wed, Jun 20, 2012 at 2:22 PM, Dmitry Vyukov <dvyukov@google.com> wrote: > > > On Wed, Jun 20, 2012 at 5:43 AM, <samsonov@google.com> wrote: > >> Reviewers: dvyukov, kcc1, >> >> >> >> http://codereview.appspot.com/**6303097/diff/1/sanitizer_** >> common/sanitizer_common.h<http://codereview.appspot.com/6303097/diff/1/sanitizer_common/sanitizer_common.h> >> File sanitizer_common/sanitizer_**common.h (right): >> >> http://codereview.appspot.com/**6303097/diff/1/sanitizer_** >> common/sanitizer_common.h#**newcode82<http://codereview.appspot.com/6303097/diff/1/sanitizer_common/sanitizer_common.h#newcode82> >> sanitizer_common/sanitizer_**common.h:82: // Debug info routines >> On 2012/06/18 18:18:38, dvyukov wrote: >> >>> Looks like a wrong place for it. It's not quite "common". Can it live >>> >> in >> >>> symbolizer? >>> >> >> Done. >> >> >> http://codereview.appspot.com/**6303097/diff/1/sanitizer_** >> common/sanitizer_symbolizer.cc<http://codereview.appspot.com/6303097/diff/1/sanitizer_common/sanitizer_symbolizer.cc> >> File sanitizer_common/sanitizer_**symbolizer.cc (right): >> >> http://codereview.appspot.com/**6303097/diff/1/sanitizer_** >> common/sanitizer_symbolizer.**cc#newcode39<http://codereview.appspot.com/6303097/diff/1/sanitizer_common/sanitizer_symbolizer.cc#newcode39> >> sanitizer_common/sanitizer_**symbolizer.cc:39: DWARFSection debug_info; >> On 2012/06/18 18:18:38, dvyukov wrote: >> >>> You don't need them here. >>> You need them only in ctor. >>> >> >> Done. >> >> >> http://codereview.appspot.com/**6303097/diff/1/sanitizer_** >> common/sanitizer_symbolizer.**cc#newcode46<http://codereview.appspot.com/6303097/diff/1/sanitizer_common/sanitizer_symbolizer.cc#newcode46> >> sanitizer_common/sanitizer_**symbolizer.cc:46: >> ReadFileToBuffer(module_name, (char**)&addr, &size, kMaxModuleSize); >> On 2012/06/18 18:18:38, dvyukov wrote: >> >>> That's a bad idea. You need to use mmap(). Otherwise you will soak up >>> >> into >> >>> memory GBs of uninteresting data. >>> >> >> Done. >> >> >> http://codereview.appspot.com/**6303097/diff/1/sanitizer_** >> common/sanitizer_symbolizer.**cc#newcode97<http://codereview.appspot.com/6303097/diff/1/sanitizer_common/sanitizer_symbolizer.cc#newcode97> >> sanitizer_common/sanitizer_**symbolizer.cc:97: while >> (proc_maps.Next(&start, &end, &offset, module_name, >> On 2012/06/18 18:18:38, dvyukov wrote: >> >>> This ain't gonna work. >>> You need 1 DIContext per *module*. >>> >> Damn, you're right here. Rewrote this part (use loookup by name to find >> a debug info context for a module or create one). >> >> >> This thing will give you N DIContexts per module (code, code, data, >>> >> data), and >> >>> you won't find .bss/.tbss sections at all, because they are no mapped >>> >> from the >> >>> file. So you will have N times memory consumption + will be unable to >>> >> symbolize >> >>> some data. >>> >> >> Probably I lack understanding here. >> OK, what can I do about .bss/.tbss? If they are not mapped into the >> memory, then how can I get (virtual) address of data in these sections >> passed to symbolizer? > > > > They are mapped, but not from the file. You can't figure out what it is by > looking at /proc/self/maps. It's just an anonymous mapping there. > In tsan I use dl_iterate_phdr. It provides all the necessary in > information (and w/o text parsing). > In ASan we used dl_iterate_phdr as well, but later removed it. Probably kcc@or eugenis@could comment on the reasons. > >> >> >> http://codereview.appspot.com/**6303097/diff/1/sanitizer_** >> common/sanitizer_symbolizer.**cc#newcode144<http://codereview.appspot.com/6303097/diff/1/sanitizer_common/sanitizer_symbolizer.cc#newcode144> >> sanitizer_common/sanitizer_**symbolizer.cc:144: >> module->di_context->**getFileLineInfo(module_offset, &info->file, >> On 2012/06/18 18:18:38, dvyukov wrote: >> >>> I believe eventually you will want these 2 function calls to be 1 >>> >> function call. >> >>> Because llvm's DIContext::SymbolizeCode() is 1 function call. >>> >> >> I don't think returning file/line info and function name in one call to >> DIContext is the right thing to do (as Benjamin mentioned, these >> operations are too different in DWARF). Maybe I'll change my mind, >> though. >> > > DIContext has nothing to do with DWARF (well, almost). It's an abstract > symbolizer interface. Think what would be a good interface for an abstract > symbolizer. Then implement it for DWARF. > For now I think it's not good to provide all the debug info data for address in one bundle (user may just not need all this, so why calculate it, esp. if it's expensive). It's all just words until there is some actual code, though. > > >> >> Description: >> This patch extends symbolizer code. In order to symbolize a module, a >> symbolizer maps it into memory and obtains pointers to necessary debug >> info sections (last part is implemented on Linux only). >> >> Later these pointers can be passed to constructor of DWARF >> context-in-memory from LLVM DebugInfo lib. >> >> >> Please review this at http://codereview.appspot.com/**6303097/<http://codereview.appspot.com/6303097/> >> >> Affected files: >> M asan/asan_stack.cc >> M sanitizer_common/sanitizer_**common.h >> M sanitizer_common/sanitizer_**linux.cc >> M sanitizer_common/sanitizer_**mac.cc >> M sanitizer_common/sanitizer_**posix.cc >> M sanitizer_common/sanitizer_**procmaps.h >> M sanitizer_common/sanitizer_**symbolizer.cc >> M sanitizer_common/sanitizer_**symbolizer.h >> M sanitizer_common/sanitizer_**win.cc >> > -- Alexey Samsonov, MSK
Sign in to reply to this message.
On Wed, Jun 20, 2012 at 2:30 PM, Alexey Samsonov <samsonov@google.com>wrote: > > > On Wed, Jun 20, 2012 at 2:22 PM, Dmitry Vyukov <dvyukov@google.com> wrote: > >> >> >> On Wed, Jun 20, 2012 at 5:43 AM, <samsonov@google.com> wrote: >> >>> Reviewers: dvyukov, kcc1, >>> >>> >>> >>> http://codereview.appspot.com/**6303097/diff/1/sanitizer_** >>> common/sanitizer_common.h<http://codereview.appspot.com/6303097/diff/1/sanitizer_common/sanitizer_common.h> >>> File sanitizer_common/sanitizer_**common.h (right): >>> >>> http://codereview.appspot.com/**6303097/diff/1/sanitizer_** >>> common/sanitizer_common.h#**newcode82<http://codereview.appspot.com/6303097/diff/1/sanitizer_common/sanitizer_common.h#newcode82> >>> sanitizer_common/sanitizer_**common.h:82: // Debug info routines >>> On 2012/06/18 18:18:38, dvyukov wrote: >>> >>>> Looks like a wrong place for it. It's not quite "common". Can it live >>>> >>> in >>> >>>> symbolizer? >>>> >>> >>> Done. >>> >>> >>> http://codereview.appspot.com/**6303097/diff/1/sanitizer_** >>> common/sanitizer_symbolizer.cc<http://codereview.appspot.com/6303097/diff/1/sanitizer_common/sanitizer_symbolizer.cc> >>> File sanitizer_common/sanitizer_**symbolizer.cc (right): >>> >>> http://codereview.appspot.com/**6303097/diff/1/sanitizer_** >>> common/sanitizer_symbolizer.**cc#newcode39<http://codereview.appspot.com/6303097/diff/1/sanitizer_common/sanitizer_symbolizer.cc#newcode39> >>> sanitizer_common/sanitizer_**symbolizer.cc:39: DWARFSection debug_info; >>> On 2012/06/18 18:18:38, dvyukov wrote: >>> >>>> You don't need them here. >>>> You need them only in ctor. >>>> >>> >>> Done. >>> >>> >>> http://codereview.appspot.com/**6303097/diff/1/sanitizer_** >>> common/sanitizer_symbolizer.**cc#newcode46<http://codereview.appspot.com/6303097/diff/1/sanitizer_common/sanitizer_symbolizer.cc#newcode46> >>> sanitizer_common/sanitizer_**symbolizer.cc:46: >>> ReadFileToBuffer(module_name, (char**)&addr, &size, kMaxModuleSize); >>> On 2012/06/18 18:18:38, dvyukov wrote: >>> >>>> That's a bad idea. You need to use mmap(). Otherwise you will soak up >>>> >>> into >>> >>>> memory GBs of uninteresting data. >>>> >>> >>> Done. >>> >>> >>> http://codereview.appspot.com/**6303097/diff/1/sanitizer_** >>> common/sanitizer_symbolizer.**cc#newcode97<http://codereview.appspot.com/6303097/diff/1/sanitizer_common/sanitizer_symbolizer.cc#newcode97> >>> sanitizer_common/sanitizer_**symbolizer.cc:97: while >>> (proc_maps.Next(&start, &end, &offset, module_name, >>> On 2012/06/18 18:18:38, dvyukov wrote: >>> >>>> This ain't gonna work. >>>> You need 1 DIContext per *module*. >>>> >>> Damn, you're right here. Rewrote this part (use loookup by name to find >>> a debug info context for a module or create one). >>> >>> >>> This thing will give you N DIContexts per module (code, code, data, >>>> >>> data), and >>> >>>> you won't find .bss/.tbss sections at all, because they are no mapped >>>> >>> from the >>> >>>> file. So you will have N times memory consumption + will be unable to >>>> >>> symbolize >>> >>>> some data. >>>> >>> >>> Probably I lack understanding here. >>> OK, what can I do about .bss/.tbss? If they are not mapped into the >>> memory, then how can I get (virtual) address of data in these sections >>> passed to symbolizer? >> >> >> >> They are mapped, but not from the file. You can't figure out what it is >> by looking at /proc/self/maps. It's just an anonymous mapping there. >> In tsan I use dl_iterate_phdr. It provides all the necessary in >> information (and w/o text parsing). >> > > In ASan we used dl_iterate_phdr as well, but later removed it. Probably > kcc@ or eugenis@ could comment on the reasons. > It had some bug which I did not manage to understand. Besides, it does not work on Android. > > >> >>> >>> >>> http://codereview.appspot.com/**6303097/diff/1/sanitizer_** >>> common/sanitizer_symbolizer.**cc#newcode144<http://codereview.appspot.com/6303097/diff/1/sanitizer_common/sanitizer_symbolizer.cc#newcode144> >>> sanitizer_common/sanitizer_**symbolizer.cc:144: >>> module->di_context->**getFileLineInfo(module_offset, &info->file, >>> On 2012/06/18 18:18:38, dvyukov wrote: >>> >>>> I believe eventually you will want these 2 function calls to be 1 >>>> >>> function call. >>> >>>> Because llvm's DIContext::SymbolizeCode() is 1 function call. >>>> >>> >>> I don't think returning file/line info and function name in one call to >>> DIContext is the right thing to do (as Benjamin mentioned, these >>> operations are too different in DWARF). Maybe I'll change my mind, >>> though. >>> >> >> DIContext has nothing to do with DWARF (well, almost). It's an abstract >> symbolizer interface. Think what would be a good interface for an abstract >> symbolizer. Then implement it for DWARF. >> > > For now I think it's not good to provide all the debug info data for > address in one bundle (user may just not need all this, so why calculate > it, esp. if it's expensive). > It's all just words until there is some actual code, though. > >> >> >>> >>> Description: >>> This patch extends symbolizer code. In order to symbolize a module, a >>> symbolizer maps it into memory and obtains pointers to necessary debug >>> info sections (last part is implemented on Linux only). >>> >>> Later these pointers can be passed to constructor of DWARF >>> context-in-memory from LLVM DebugInfo lib. >>> >>> >>> Please review this at http://codereview.appspot.com/**6303097/<http://codereview.appspot.com/6303097/> >>> >>> Affected files: >>> M asan/asan_stack.cc >>> M sanitizer_common/sanitizer_**common.h >>> M sanitizer_common/sanitizer_**linux.cc >>> M sanitizer_common/sanitizer_**mac.cc >>> M sanitizer_common/sanitizer_**posix.cc >>> M sanitizer_common/sanitizer_**procmaps.h >>> M sanitizer_common/sanitizer_**symbolizer.cc >>> M sanitizer_common/sanitizer_**symbolizer.h >>> M sanitizer_common/sanitizer_**win.cc >>> >> > > > -- > Alexey Samsonov, MSK > >
Sign in to reply to this message.
On Wed, Jun 20, 2012 at 6:30 AM, Alexey Samsonov <samsonov@google.com>wrote: > http://codereview.appspot.com/**6303097/diff/1/sanitizer_** >>> common/sanitizer_common.h<http://codereview.appspot.com/6303097/diff/1/sanitizer_common/sanitizer_common.h> >>> File sanitizer_common/sanitizer_**common.h (right): >>> >>> http://codereview.appspot.com/**6303097/diff/1/sanitizer_** >>> common/sanitizer_common.h#**newcode82<http://codereview.appspot.com/6303097/diff/1/sanitizer_common/sanitizer_common.h#newcode82> >>> sanitizer_common/sanitizer_**common.h:82: // Debug info routines >>> On 2012/06/18 18:18:38, dvyukov wrote: >>> >>>> Looks like a wrong place for it. It's not quite "common". Can it live >>>> >>> in >>> >>>> symbolizer? >>>> >>> >>> Done. >>> >>> >>> http://codereview.appspot.com/**6303097/diff/1/sanitizer_** >>> common/sanitizer_symbolizer.cc<http://codereview.appspot.com/6303097/diff/1/sanitizer_common/sanitizer_symbolizer.cc> >>> File sanitizer_common/sanitizer_**symbolizer.cc (right): >>> >>> http://codereview.appspot.com/**6303097/diff/1/sanitizer_** >>> common/sanitizer_symbolizer.**cc#newcode39<http://codereview.appspot.com/6303097/diff/1/sanitizer_common/sanitizer_symbolizer.cc#newcode39> >>> sanitizer_common/sanitizer_**symbolizer.cc:39: DWARFSection debug_info; >>> On 2012/06/18 18:18:38, dvyukov wrote: >>> >>>> You don't need them here. >>>> You need them only in ctor. >>>> >>> >>> Done. >>> >>> >>> http://codereview.appspot.com/**6303097/diff/1/sanitizer_** >>> common/sanitizer_symbolizer.**cc#newcode46<http://codereview.appspot.com/6303097/diff/1/sanitizer_common/sanitizer_symbolizer.cc#newcode46> >>> sanitizer_common/sanitizer_**symbolizer.cc:46: >>> ReadFileToBuffer(module_name, (char**)&addr, &size, kMaxModuleSize); >>> On 2012/06/18 18:18:38, dvyukov wrote: >>> >>>> That's a bad idea. You need to use mmap(). Otherwise you will soak up >>>> >>> into >>> >>>> memory GBs of uninteresting data. >>>> >>> >>> Done. >>> >>> >>> http://codereview.appspot.com/**6303097/diff/1/sanitizer_** >>> common/sanitizer_symbolizer.**cc#newcode97<http://codereview.appspot.com/6303097/diff/1/sanitizer_common/sanitizer_symbolizer.cc#newcode97> >>> sanitizer_common/sanitizer_**symbolizer.cc:97: while >>> (proc_maps.Next(&start, &end, &offset, module_name, >>> On 2012/06/18 18:18:38, dvyukov wrote: >>> >>>> This ain't gonna work. >>>> You need 1 DIContext per *module*. >>>> >>> Damn, you're right here. Rewrote this part (use loookup by name to find >>> a debug info context for a module or create one). >>> >>> >>> This thing will give you N DIContexts per module (code, code, data, >>>> >>> data), and >>> >>>> you won't find .bss/.tbss sections at all, because they are no mapped >>>> >>> from the >>> >>>> file. So you will have N times memory consumption + will be unable to >>>> >>> symbolize >>> >>>> some data. >>>> >>> >>> Probably I lack understanding here. >>> OK, what can I do about .bss/.tbss? If they are not mapped into the >>> memory, then how can I get (virtual) address of data in these sections >>> passed to symbolizer? >> >> >> >> They are mapped, but not from the file. You can't figure out what it is >> by looking at /proc/self/maps. It's just an anonymous mapping there. >> In tsan I use dl_iterate_phdr. It provides all the necessary in >> information (and w/o text parsing). >> > > In ASan we used dl_iterate_phdr as well, but later removed it. Probably > kcc@ or eugenis@ could comment on the reasons. > > >> >>> >>> >>> http://codereview.appspot.com/**6303097/diff/1/sanitizer_** >>> common/sanitizer_symbolizer.**cc#newcode144<http://codereview.appspot.com/6303097/diff/1/sanitizer_common/sanitizer_symbolizer.cc#newcode144> >>> sanitizer_common/sanitizer_**symbolizer.cc:144: >>> module->di_context->**getFileLineInfo(module_offset, &info->file, >>> On 2012/06/18 18:18:38, dvyukov wrote: >>> >>>> I believe eventually you will want these 2 function calls to be 1 >>>> >>> function call. >>> >>>> Because llvm's DIContext::SymbolizeCode() is 1 function call. >>>> >>> >>> I don't think returning file/line info and function name in one call to >>> DIContext is the right thing to do (as Benjamin mentioned, these >>> operations are too different in DWARF). Maybe I'll change my mind, >>> though. >>> >> >> DIContext has nothing to do with DWARF (well, almost). It's an abstract >> symbolizer interface. Think what would be a good interface for an abstract >> symbolizer. Then implement it for DWARF. >> > > For now I think it's not good to provide all the debug info data for > address in one bundle (user may just not need all this, so why calculate > it, esp. if it's expensive). > It's all just words until there is some actual code, though. > lldb uses flags that says what info you want, e.g. "give me function name and line number", "give me symbol name", "give me inlining info", etc.
Sign in to reply to this message.
On Wed, Jun 20, 2012 at 6:30 AM, Alexey Samsonov <samsonov@google.com>wrote: > http://codereview.appspot.com/**6303097/diff/1/sanitizer_** >>> common/sanitizer_common.h<http://codereview.appspot.com/6303097/diff/1/sanitizer_common/sanitizer_common.h> >>> File sanitizer_common/sanitizer_**common.h (right): >>> >>> http://codereview.appspot.com/**6303097/diff/1/sanitizer_** >>> common/sanitizer_common.h#**newcode82<http://codereview.appspot.com/6303097/diff/1/sanitizer_common/sanitizer_common.h#newcode82> >>> sanitizer_common/sanitizer_**common.h:82: // Debug info routines >>> On 2012/06/18 18:18:38, dvyukov wrote: >>> >>>> Looks like a wrong place for it. It's not quite "common". Can it live >>>> >>> in >>> >>>> symbolizer? >>>> >>> >>> Done. >>> >>> >>> http://codereview.appspot.com/**6303097/diff/1/sanitizer_** >>> common/sanitizer_symbolizer.cc<http://codereview.appspot.com/6303097/diff/1/sanitizer_common/sanitizer_symbolizer.cc> >>> File sanitizer_common/sanitizer_**symbolizer.cc (right): >>> >>> http://codereview.appspot.com/**6303097/diff/1/sanitizer_** >>> common/sanitizer_symbolizer.**cc#newcode39<http://codereview.appspot.com/6303097/diff/1/sanitizer_common/sanitizer_symbolizer.cc#newcode39> >>> sanitizer_common/sanitizer_**symbolizer.cc:39: DWARFSection debug_info; >>> On 2012/06/18 18:18:38, dvyukov wrote: >>> >>>> You don't need them here. >>>> You need them only in ctor. >>>> >>> >>> Done. >>> >>> >>> http://codereview.appspot.com/**6303097/diff/1/sanitizer_** >>> common/sanitizer_symbolizer.**cc#newcode46<http://codereview.appspot.com/6303097/diff/1/sanitizer_common/sanitizer_symbolizer.cc#newcode46> >>> sanitizer_common/sanitizer_**symbolizer.cc:46: >>> ReadFileToBuffer(module_name, (char**)&addr, &size, kMaxModuleSize); >>> On 2012/06/18 18:18:38, dvyukov wrote: >>> >>>> That's a bad idea. You need to use mmap(). Otherwise you will soak up >>>> >>> into >>> >>>> memory GBs of uninteresting data. >>>> >>> >>> Done. >>> >>> >>> http://codereview.appspot.com/**6303097/diff/1/sanitizer_** >>> common/sanitizer_symbolizer.**cc#newcode97<http://codereview.appspot.com/6303097/diff/1/sanitizer_common/sanitizer_symbolizer.cc#newcode97> >>> sanitizer_common/sanitizer_**symbolizer.cc:97: while >>> (proc_maps.Next(&start, &end, &offset, module_name, >>> On 2012/06/18 18:18:38, dvyukov wrote: >>> >>>> This ain't gonna work. >>>> You need 1 DIContext per *module*. >>>> >>> Damn, you're right here. Rewrote this part (use loookup by name to find >>> a debug info context for a module or create one). >>> >>> >>> This thing will give you N DIContexts per module (code, code, data, >>>> >>> data), and >>> >>>> you won't find .bss/.tbss sections at all, because they are no mapped >>>> >>> from the >>> >>>> file. So you will have N times memory consumption + will be unable to >>>> >>> symbolize >>> >>>> some data. >>>> >>> >>> Probably I lack understanding here. >>> OK, what can I do about .bss/.tbss? If they are not mapped into the >>> memory, then how can I get (virtual) address of data in these sections >>> passed to symbolizer? >> >> >> >> They are mapped, but not from the file. You can't figure out what it is >> by looking at /proc/self/maps. It's just an anonymous mapping there. >> In tsan I use dl_iterate_phdr. It provides all the necessary in >> information (and w/o text parsing). >> > > In ASan we used dl_iterate_phdr as well, but later removed it. Probably > kcc@ or eugenis@ could comment on the reasons. > > >> >>> >>> >>> http://codereview.appspot.com/**6303097/diff/1/sanitizer_** >>> common/sanitizer_symbolizer.**cc#newcode144<http://codereview.appspot.com/6303097/diff/1/sanitizer_common/sanitizer_symbolizer.cc#newcode144> >>> sanitizer_common/sanitizer_**symbolizer.cc:144: >>> module->di_context->**getFileLineInfo(module_offset, &info->file, >>> On 2012/06/18 18:18:38, dvyukov wrote: >>> >>>> I believe eventually you will want these 2 function calls to be 1 >>>> >>> function call. >>> >>>> Because llvm's DIContext::SymbolizeCode() is 1 function call. >>>> >>> >>> I don't think returning file/line info and function name in one call to >>> DIContext is the right thing to do (as Benjamin mentioned, these >>> operations are too different in DWARF). Maybe I'll change my mind, >>> though. >>> >> >> DIContext has nothing to do with DWARF (well, almost). It's an abstract >> symbolizer interface. Think what would be a good interface for an abstract >> symbolizer. Then implement it for DWARF. >> > > For now I think it's not good to provide all the debug info data for > address in one bundle (user may just not need all this, so why calculate > it, esp. if it's expensive). > It's all just words until there is some actual code, though. > lldb uses flags that says what info you want, e.g. give
Sign in to reply to this message.
On Wed, Jun 20, 2012 at 6:35 AM, Kostya Serebryany <kcc@google.com> wrote: > >>>> http://codereview.appspot.com/**6303097/diff/1/sanitizer_** >>>> common/sanitizer_common.h<http://codereview.appspot.com/6303097/diff/1/sanitizer_common/sanitizer_common.h> >>>> File sanitizer_common/sanitizer_**common.h (right): >>>> >>>> http://codereview.appspot.com/**6303097/diff/1/sanitizer_** >>>> common/sanitizer_common.h#**newcode82<http://codereview.appspot.com/6303097/diff/1/sanitizer_common/sanitizer_common.h#newcode82> >>>> sanitizer_common/sanitizer_**common.h:82: // Debug info routines >>>> On 2012/06/18 18:18:38, dvyukov wrote: >>>> >>>>> Looks like a wrong place for it. It's not quite "common". Can it live >>>>> >>>> in >>>> >>>>> symbolizer? >>>>> >>>> >>>> Done. >>>> >>>> >>>> http://codereview.appspot.com/**6303097/diff/1/sanitizer_** >>>> common/sanitizer_symbolizer.cc<http://codereview.appspot.com/6303097/diff/1/sanitizer_common/sanitizer_symbolizer.cc> >>>> File sanitizer_common/sanitizer_**symbolizer.cc (right): >>>> >>>> http://codereview.appspot.com/**6303097/diff/1/sanitizer_** >>>> common/sanitizer_symbolizer.**cc#newcode39<http://codereview.appspot.com/6303097/diff/1/sanitizer_common/sanitizer_symbolizer.cc#newcode39> >>>> sanitizer_common/sanitizer_**symbolizer.cc:39: DWARFSection debug_info; >>>> On 2012/06/18 18:18:38, dvyukov wrote: >>>> >>>>> You don't need them here. >>>>> You need them only in ctor. >>>>> >>>> >>>> Done. >>>> >>>> >>>> http://codereview.appspot.com/**6303097/diff/1/sanitizer_** >>>> common/sanitizer_symbolizer.**cc#newcode46<http://codereview.appspot.com/6303097/diff/1/sanitizer_common/sanitizer_symbolizer.cc#newcode46> >>>> sanitizer_common/sanitizer_**symbolizer.cc:46: >>>> ReadFileToBuffer(module_name, (char**)&addr, &size, kMaxModuleSize); >>>> On 2012/06/18 18:18:38, dvyukov wrote: >>>> >>>>> That's a bad idea. You need to use mmap(). Otherwise you will soak up >>>>> >>>> into >>>> >>>>> memory GBs of uninteresting data. >>>>> >>>> >>>> Done. >>>> >>>> >>>> http://codereview.appspot.com/**6303097/diff/1/sanitizer_** >>>> common/sanitizer_symbolizer.**cc#newcode97<http://codereview.appspot.com/6303097/diff/1/sanitizer_common/sanitizer_symbolizer.cc#newcode97> >>>> sanitizer_common/sanitizer_**symbolizer.cc:97: while >>>> (proc_maps.Next(&start, &end, &offset, module_name, >>>> On 2012/06/18 18:18:38, dvyukov wrote: >>>> >>>>> This ain't gonna work. >>>>> You need 1 DIContext per *module*. >>>>> >>>> Damn, you're right here. Rewrote this part (use loookup by name to find >>>> a debug info context for a module or create one). >>>> >>>> >>>> This thing will give you N DIContexts per module (code, code, data, >>>>> >>>> data), and >>>> >>>>> you won't find .bss/.tbss sections at all, because they are no mapped >>>>> >>>> from the >>>> >>>>> file. So you will have N times memory consumption + will be unable to >>>>> >>>> symbolize >>>> >>>>> some data. >>>>> >>>> >>>> Probably I lack understanding here. >>>> OK, what can I do about .bss/.tbss? If they are not mapped into the >>>> memory, then how can I get (virtual) address of data in these sections >>>> passed to symbolizer? >>> >>> >>> >>> They are mapped, but not from the file. You can't figure out what it is >>> by looking at /proc/self/maps. It's just an anonymous mapping there. >>> In tsan I use dl_iterate_phdr. It provides all the necessary in >>> information (and w/o text parsing). >>> >> >> In ASan we used dl_iterate_phdr as well, but later removed it. Probably >> kcc@ or eugenis@ could comment on the reasons. >> > > It had some bug which I did not manage to understand. > Besides, it does not work on Android. > dl_iterate_phdr works for linux w/o any problems. I can understand if for Android we will have something else, because, well, it *is* platform-dependent. It's /proc/self/maps that does not work. It gives you not the info you are looking for. To some degree it can be fixed, but it won't provide e.g. info about .bss/.tbss.
Sign in to reply to this message.
On Wed, Jun 20, 2012 at 2:44 PM, Dmitry Vyukov <dvyukov@google.com> wrote: > On Wed, Jun 20, 2012 at 6:35 AM, Kostya Serebryany <kcc@google.com> wrote: > >> >>>>> http://codereview.appspot.com/**6303097/diff/1/sanitizer_** >>>>> common/sanitizer_common.h<http://codereview.appspot.com/6303097/diff/1/sanitizer_common/sanitizer_common.h> >>>>> File sanitizer_common/sanitizer_**common.h (right): >>>>> >>>>> http://codereview.appspot.com/**6303097/diff/1/sanitizer_** >>>>> common/sanitizer_common.h#**newcode82<http://codereview.appspot.com/6303097/diff/1/sanitizer_common/sanitizer_common.h#newcode82> >>>>> sanitizer_common/sanitizer_**common.h:82: // Debug info routines >>>>> On 2012/06/18 18:18:38, dvyukov wrote: >>>>> >>>>>> Looks like a wrong place for it. It's not quite "common". Can it live >>>>>> >>>>> in >>>>> >>>>>> symbolizer? >>>>>> >>>>> >>>>> Done. >>>>> >>>>> >>>>> http://codereview.appspot.com/**6303097/diff/1/sanitizer_** >>>>> common/sanitizer_symbolizer.cc<http://codereview.appspot.com/6303097/diff/1/sanitizer_common/sanitizer_symbolizer.cc> >>>>> File sanitizer_common/sanitizer_**symbolizer.cc (right): >>>>> >>>>> http://codereview.appspot.com/**6303097/diff/1/sanitizer_** >>>>> common/sanitizer_symbolizer.**cc#newcode39<http://codereview.appspot.com/6303097/diff/1/sanitizer_common/sanitizer_symbolizer.cc#newcode39> >>>>> sanitizer_common/sanitizer_**symbolizer.cc:39: DWARFSection >>>>> debug_info; >>>>> On 2012/06/18 18:18:38, dvyukov wrote: >>>>> >>>>>> You don't need them here. >>>>>> You need them only in ctor. >>>>>> >>>>> >>>>> Done. >>>>> >>>>> >>>>> http://codereview.appspot.com/**6303097/diff/1/sanitizer_** >>>>> common/sanitizer_symbolizer.**cc#newcode46<http://codereview.appspot.com/6303097/diff/1/sanitizer_common/sanitizer_symbolizer.cc#newcode46> >>>>> sanitizer_common/sanitizer_**symbolizer.cc:46: >>>>> ReadFileToBuffer(module_name, (char**)&addr, &size, kMaxModuleSize); >>>>> On 2012/06/18 18:18:38, dvyukov wrote: >>>>> >>>>>> That's a bad idea. You need to use mmap(). Otherwise you will soak up >>>>>> >>>>> into >>>>> >>>>>> memory GBs of uninteresting data. >>>>>> >>>>> >>>>> Done. >>>>> >>>>> >>>>> http://codereview.appspot.com/**6303097/diff/1/sanitizer_** >>>>> common/sanitizer_symbolizer.**cc#newcode97<http://codereview.appspot.com/6303097/diff/1/sanitizer_common/sanitizer_symbolizer.cc#newcode97> >>>>> sanitizer_common/sanitizer_**symbolizer.cc:97: while >>>>> (proc_maps.Next(&start, &end, &offset, module_name, >>>>> On 2012/06/18 18:18:38, dvyukov wrote: >>>>> >>>>>> This ain't gonna work. >>>>>> You need 1 DIContext per *module*. >>>>>> >>>>> Damn, you're right here. Rewrote this part (use loookup by name to find >>>>> a debug info context for a module or create one). >>>>> >>>>> >>>>> This thing will give you N DIContexts per module (code, code, data, >>>>>> >>>>> data), and >>>>> >>>>>> you won't find .bss/.tbss sections at all, because they are no mapped >>>>>> >>>>> from the >>>>> >>>>>> file. So you will have N times memory consumption + will be unable to >>>>>> >>>>> symbolize >>>>> >>>>>> some data. >>>>>> >>>>> >>>>> Probably I lack understanding here. >>>>> OK, what can I do about .bss/.tbss? If they are not mapped into the >>>>> memory, then how can I get (virtual) address of data in these sections >>>>> passed to symbolizer? >>>> >>>> >>>> >>>> They are mapped, but not from the file. You can't figure out what it is >>>> by looking at /proc/self/maps. It's just an anonymous mapping there. >>>> In tsan I use dl_iterate_phdr. It provides all the necessary in >>>> information (and w/o text parsing). >>>> >>> >>> In ASan we used dl_iterate_phdr as well, but later removed it. Probably >>> kcc@ or eugenis@ could comment on the reasons. >>> >> >> It had some bug which I did not manage to understand. >> Besides, it does not work on Android. >> > > dl_iterate_phdr works for linux w/o any problems. > It did not work for shared libraries. Perhaps there was a bug in my code that used it. --kcc > I can understand if for Android we will have something else, because, > well, it *is* platform-dependent. > It's /proc/self/maps that does not work. It gives you not the info you are > looking for. To some degree it can be fixed, but it won't provide e.g. info > about .bss/.tbss. > >
Sign in to reply to this message.
On Wed, Jun 20, 2012 at 2:44 PM, Dmitry Vyukov <dvyukov@google.com> wrote: > On Wed, Jun 20, 2012 at 6:35 AM, Kostya Serebryany <kcc@google.com> wrote: > >> >>>>> http://codereview.appspot.com/**6303097/diff/1/sanitizer_** >>>>> common/sanitizer_common.h<http://codereview.appspot.com/6303097/diff/1/sanitizer_common/sanitizer_common.h> >>>>> File sanitizer_common/sanitizer_**common.h (right): >>>>> >>>>> http://codereview.appspot.com/**6303097/diff/1/sanitizer_** >>>>> common/sanitizer_common.h#**newcode82<http://codereview.appspot.com/6303097/diff/1/sanitizer_common/sanitizer_common.h#newcode82> >>>>> sanitizer_common/sanitizer_**common.h:82: // Debug info routines >>>>> On 2012/06/18 18:18:38, dvyukov wrote: >>>>> >>>>>> Looks like a wrong place for it. It's not quite "common". Can it live >>>>>> >>>>> in >>>>> >>>>>> symbolizer? >>>>>> >>>>> >>>>> Done. >>>>> >>>>> >>>>> http://codereview.appspot.com/**6303097/diff/1/sanitizer_** >>>>> common/sanitizer_symbolizer.cc<http://codereview.appspot.com/6303097/diff/1/sanitizer_common/sanitizer_symbolizer.cc> >>>>> File sanitizer_common/sanitizer_**symbolizer.cc (right): >>>>> >>>>> http://codereview.appspot.com/**6303097/diff/1/sanitizer_** >>>>> common/sanitizer_symbolizer.**cc#newcode39<http://codereview.appspot.com/6303097/diff/1/sanitizer_common/sanitizer_symbolizer.cc#newcode39> >>>>> sanitizer_common/sanitizer_**symbolizer.cc:39: DWARFSection >>>>> debug_info; >>>>> On 2012/06/18 18:18:38, dvyukov wrote: >>>>> >>>>>> You don't need them here. >>>>>> You need them only in ctor. >>>>>> >>>>> >>>>> Done. >>>>> >>>>> >>>>> http://codereview.appspot.com/**6303097/diff/1/sanitizer_** >>>>> common/sanitizer_symbolizer.**cc#newcode46<http://codereview.appspot.com/6303097/diff/1/sanitizer_common/sanitizer_symbolizer.cc#newcode46> >>>>> sanitizer_common/sanitizer_**symbolizer.cc:46: >>>>> ReadFileToBuffer(module_name, (char**)&addr, &size, kMaxModuleSize); >>>>> On 2012/06/18 18:18:38, dvyukov wrote: >>>>> >>>>>> That's a bad idea. You need to use mmap(). Otherwise you will soak up >>>>>> >>>>> into >>>>> >>>>>> memory GBs of uninteresting data. >>>>>> >>>>> >>>>> Done. >>>>> >>>>> >>>>> http://codereview.appspot.com/**6303097/diff/1/sanitizer_** >>>>> common/sanitizer_symbolizer.**cc#newcode97<http://codereview.appspot.com/6303097/diff/1/sanitizer_common/sanitizer_symbolizer.cc#newcode97> >>>>> sanitizer_common/sanitizer_**symbolizer.cc:97: while >>>>> (proc_maps.Next(&start, &end, &offset, module_name, >>>>> On 2012/06/18 18:18:38, dvyukov wrote: >>>>> >>>>>> This ain't gonna work. >>>>>> You need 1 DIContext per *module*. >>>>>> >>>>> Damn, you're right here. Rewrote this part (use loookup by name to find >>>>> a debug info context for a module or create one). >>>>> >>>>> >>>>> This thing will give you N DIContexts per module (code, code, data, >>>>>> >>>>> data), and >>>>> >>>>>> you won't find .bss/.tbss sections at all, because they are no mapped >>>>>> >>>>> from the >>>>> >>>>>> file. So you will have N times memory consumption + will be unable to >>>>>> >>>>> symbolize >>>>> >>>>>> some data. >>>>>> >>>>> >>>>> Probably I lack understanding here. >>>>> OK, what can I do about .bss/.tbss? If they are not mapped into the >>>>> memory, then how can I get (virtual) address of data in these sections >>>>> passed to symbolizer? >>>> >>>> >>>> >>>> They are mapped, but not from the file. You can't figure out what it is >>>> by looking at /proc/self/maps. It's just an anonymous mapping there. >>>> In tsan I use dl_iterate_phdr. It provides all the necessary in >>>> information (and w/o text parsing). >>>> >>> >>> In ASan we used dl_iterate_phdr as well, but later removed it. Probably >>> kcc@ or eugenis@ could comment on the reasons. >>> >> >> It had some bug which I did not manage to understand. >> Besides, it does not work on Android. >> > > dl_iterate_phdr works for linux w/o any problems. > So, you suggest to try and use it once again? > I can understand if for Android we will have something else, because, > well, it *is* platform-dependent. > It's /proc/self/maps that does not work. It gives you not the info you are > looking for. To some degree it can be fixed, but it won't provide e.g. info > about .bss/.tbss. > > -- Alexey Samsonov, MSK
Sign in to reply to this message.
On Wed, Jun 20, 2012 at 4:07 PM, Alexey Samsonov <samsonov@google.com>wrote: > >>>>>> http://codereview.appspot.com/**6303097/diff/1/sanitizer_** >>>>>> common/sanitizer_common.h<http://codereview.appspot.com/6303097/diff/1/sanitizer_common/sanitizer_common.h> >>>>>> File sanitizer_common/sanitizer_**common.h (right): >>>>>> >>>>>> http://codereview.appspot.com/**6303097/diff/1/sanitizer_** >>>>>> common/sanitizer_common.h#**newcode82<http://codereview.appspot.com/6303097/diff/1/sanitizer_common/sanitizer_common.h#newcode82> >>>>>> sanitizer_common/sanitizer_**common.h:82: // Debug info routines >>>>>> On 2012/06/18 18:18:38, dvyukov wrote: >>>>>> >>>>>>> Looks like a wrong place for it. It's not quite "common". Can it live >>>>>>> >>>>>> in >>>>>> >>>>>>> symbolizer? >>>>>>> >>>>>> >>>>>> Done. >>>>>> >>>>>> >>>>>> http://codereview.appspot.com/**6303097/diff/1/sanitizer_** >>>>>> common/sanitizer_symbolizer.cc<http://codereview.appspot.com/6303097/diff/1/sanitizer_common/sanitizer_symbolizer.cc> >>>>>> File sanitizer_common/sanitizer_**symbolizer.cc (right): >>>>>> >>>>>> http://codereview.appspot.com/**6303097/diff/1/sanitizer_** >>>>>> common/sanitizer_symbolizer.**cc#newcode39<http://codereview.appspot.com/6303097/diff/1/sanitizer_common/sanitizer_symbolizer.cc#newcode39> >>>>>> sanitizer_common/sanitizer_**symbolizer.cc:39: DWARFSection >>>>>> debug_info; >>>>>> On 2012/06/18 18:18:38, dvyukov wrote: >>>>>> >>>>>>> You don't need them here. >>>>>>> You need them only in ctor. >>>>>>> >>>>>> >>>>>> Done. >>>>>> >>>>>> >>>>>> http://codereview.appspot.com/**6303097/diff/1/sanitizer_** >>>>>> common/sanitizer_symbolizer.**cc#newcode46<http://codereview.appspot.com/6303097/diff/1/sanitizer_common/sanitizer_symbolizer.cc#newcode46> >>>>>> sanitizer_common/sanitizer_**symbolizer.cc:46: >>>>>> ReadFileToBuffer(module_name, (char**)&addr, &size, kMaxModuleSize); >>>>>> On 2012/06/18 18:18:38, dvyukov wrote: >>>>>> >>>>>>> That's a bad idea. You need to use mmap(). Otherwise you will soak up >>>>>>> >>>>>> into >>>>>> >>>>>>> memory GBs of uninteresting data. >>>>>>> >>>>>> >>>>>> Done. >>>>>> >>>>>> >>>>>> http://codereview.appspot.com/**6303097/diff/1/sanitizer_** >>>>>> common/sanitizer_symbolizer.**cc#newcode97<http://codereview.appspot.com/6303097/diff/1/sanitizer_common/sanitizer_symbolizer.cc#newcode97> >>>>>> sanitizer_common/sanitizer_**symbolizer.cc:97: while >>>>>> (proc_maps.Next(&start, &end, &offset, module_name, >>>>>> On 2012/06/18 18:18:38, dvyukov wrote: >>>>>> >>>>>>> This ain't gonna work. >>>>>>> You need 1 DIContext per *module*. >>>>>>> >>>>>> Damn, you're right here. Rewrote this part (use loookup by name to >>>>>> find >>>>>> a debug info context for a module or create one). >>>>>> >>>>>> >>>>>> This thing will give you N DIContexts per module (code, code, data, >>>>>>> >>>>>> data), and >>>>>> >>>>>>> you won't find .bss/.tbss sections at all, because they are no mapped >>>>>>> >>>>>> from the >>>>>> >>>>>>> file. So you will have N times memory consumption + will be unable to >>>>>>> >>>>>> symbolize >>>>>> >>>>>>> some data. >>>>>>> >>>>>> >>>>>> Probably I lack understanding here. >>>>>> OK, what can I do about .bss/.tbss? If they are not mapped into the >>>>>> memory, then how can I get (virtual) address of data in these sections >>>>>> passed to symbolizer? >>>>> >>>>> >>>>> >>>>> They are mapped, but not from the file. You can't figure out what it >>>>> is by looking at /proc/self/maps. It's just an anonymous mapping there. >>>>> In tsan I use dl_iterate_phdr. It provides all the necessary in >>>>> information (and w/o text parsing). >>>>> >>>> >>>> In ASan we used dl_iterate_phdr as well, but later removed it. Probably >>>> kcc@ or eugenis@ could comment on the reasons. >>>> >>> >>> It had some bug which I did not manage to understand. >>> Besides, it does not work on Android. >>> >> >> dl_iterate_phdr works for linux w/o any problems. >> > > So, you suggest to try and use it once again? > I suggest to not keep debug info in memory in 3 or more copies, and to be able to sumbolize global data. I am OK with anything that will do. I can understand if for Android we will have something else, because, well, >> it *is* platform-dependent. >> It's /proc/self/maps that does not work. It gives you not the info you >> are looking for. To some degree it can be fixed, but it won't provide e.g. >> info about .bss/.tbss. >> >> > -- > Alexey Samsonov, MSK > >
Sign in to reply to this message.
AFAIK, we just used it wrong for the shared libraries. The actual difference, as I remember it, was between PIE and non-PIE binaries. They require calculating memory offset in a different way, and addr2line interface made it quite difficult. I think the latest symbolizing scripts work fine with this. On Wed, Jun 20, 2012 at 2:52 PM, Kostya Serebryany <kcc@google.com> wrote: > > > On Wed, Jun 20, 2012 at 2:44 PM, Dmitry Vyukov <dvyukov@google.com> wrote: > >> On Wed, Jun 20, 2012 at 6:35 AM, Kostya Serebryany <kcc@google.com>wrote: >> >>> >>>>>> http://codereview.appspot.com/**6303097/diff/1/sanitizer_** >>>>>> common/sanitizer_common.h<http://codereview.appspot.com/6303097/diff/1/sanitizer_common/sanitizer_common.h> >>>>>> File sanitizer_common/sanitizer_**common.h (right): >>>>>> >>>>>> http://codereview.appspot.com/**6303097/diff/1/sanitizer_** >>>>>> common/sanitizer_common.h#**newcode82<http://codereview.appspot.com/6303097/diff/1/sanitizer_common/sanitizer_common.h#newcode82> >>>>>> sanitizer_common/sanitizer_**common.h:82: // Debug info routines >>>>>> On 2012/06/18 18:18:38, dvyukov wrote: >>>>>> >>>>>>> Looks like a wrong place for it. It's not quite "common". Can it live >>>>>>> >>>>>> in >>>>>> >>>>>>> symbolizer? >>>>>>> >>>>>> >>>>>> Done. >>>>>> >>>>>> >>>>>> http://codereview.appspot.com/**6303097/diff/1/sanitizer_** >>>>>> common/sanitizer_symbolizer.cc<http://codereview.appspot.com/6303097/diff/1/sanitizer_common/sanitizer_symbolizer.cc> >>>>>> File sanitizer_common/sanitizer_**symbolizer.cc (right): >>>>>> >>>>>> http://codereview.appspot.com/**6303097/diff/1/sanitizer_** >>>>>> common/sanitizer_symbolizer.**cc#newcode39<http://codereview.appspot.com/6303097/diff/1/sanitizer_common/sanitizer_symbolizer.cc#newcode39> >>>>>> sanitizer_common/sanitizer_**symbolizer.cc:39: DWARFSection >>>>>> debug_info; >>>>>> On 2012/06/18 18:18:38, dvyukov wrote: >>>>>> >>>>>>> You don't need them here. >>>>>>> You need them only in ctor. >>>>>>> >>>>>> >>>>>> Done. >>>>>> >>>>>> >>>>>> http://codereview.appspot.com/**6303097/diff/1/sanitizer_** >>>>>> common/sanitizer_symbolizer.**cc#newcode46<http://codereview.appspot.com/6303097/diff/1/sanitizer_common/sanitizer_symbolizer.cc#newcode46> >>>>>> sanitizer_common/sanitizer_**symbolizer.cc:46: >>>>>> ReadFileToBuffer(module_name, (char**)&addr, &size, kMaxModuleSize); >>>>>> On 2012/06/18 18:18:38, dvyukov wrote: >>>>>> >>>>>>> That's a bad idea. You need to use mmap(). Otherwise you will soak up >>>>>>> >>>>>> into >>>>>> >>>>>>> memory GBs of uninteresting data. >>>>>>> >>>>>> >>>>>> Done. >>>>>> >>>>>> >>>>>> http://codereview.appspot.com/**6303097/diff/1/sanitizer_** >>>>>> common/sanitizer_symbolizer.**cc#newcode97<http://codereview.appspot.com/6303097/diff/1/sanitizer_common/sanitizer_symbolizer.cc#newcode97> >>>>>> sanitizer_common/sanitizer_**symbolizer.cc:97: while >>>>>> (proc_maps.Next(&start, &end, &offset, module_name, >>>>>> On 2012/06/18 18:18:38, dvyukov wrote: >>>>>> >>>>>>> This ain't gonna work. >>>>>>> You need 1 DIContext per *module*. >>>>>>> >>>>>> Damn, you're right here. Rewrote this part (use loookup by name to >>>>>> find >>>>>> a debug info context for a module or create one). >>>>>> >>>>>> >>>>>> This thing will give you N DIContexts per module (code, code, data, >>>>>>> >>>>>> data), and >>>>>> >>>>>>> you won't find .bss/.tbss sections at all, because they are no mapped >>>>>>> >>>>>> from the >>>>>> >>>>>>> file. So you will have N times memory consumption + will be unable to >>>>>>> >>>>>> symbolize >>>>>> >>>>>>> some data. >>>>>>> >>>>>> >>>>>> Probably I lack understanding here. >>>>>> OK, what can I do about .bss/.tbss? If they are not mapped into the >>>>>> memory, then how can I get (virtual) address of data in these sections >>>>>> passed to symbolizer? >>>>> >>>>> >>>>> >>>>> They are mapped, but not from the file. You can't figure out what it >>>>> is by looking at /proc/self/maps. It's just an anonymous mapping there. >>>>> In tsan I use dl_iterate_phdr. It provides all the necessary in >>>>> information (and w/o text parsing). >>>>> >>>> >>>> In ASan we used dl_iterate_phdr as well, but later removed it. Probably >>>> kcc@ or eugenis@ could comment on the reasons. >>>> >>> >>> It had some bug which I did not manage to understand. >>> Besides, it does not work on Android. >>> >> >> dl_iterate_phdr works for linux w/o any problems. >> > > It did not work for shared libraries. Perhaps there was a bug in my code > that used it. > > --kcc > > > >> I can understand if for Android we will have something else, because, >> well, it *is* platform-dependent. >> It's /proc/self/maps that does not work. It gives you not the info you >> are looking for. To some degree it can be fixed, but it won't provide e.g. >> info about .bss/.tbss. >> >> >
Sign in to reply to this message.
On Thu, Jun 21, 2012 at 8:56 AM, Dmitry Vyukov <dvyukov@google.com> wrote: > On Wed, Jun 20, 2012 at 4:07 PM, Alexey Samsonov <samsonov@google.com>wrote: > >> >>>>>>> http://codereview.appspot.com/**6303097/diff/1/sanitizer_** >>>>>>> common/sanitizer_common.h<http://codereview.appspot.com/6303097/diff/1/sanitizer_common/sanitizer_common.h> >>>>>>> File sanitizer_common/sanitizer_**common.h (right): >>>>>>> >>>>>>> http://codereview.appspot.com/**6303097/diff/1/sanitizer_** >>>>>>> common/sanitizer_common.h#**newcode82<http://codereview.appspot.com/6303097/diff/1/sanitizer_common/sanitizer_common.h#newcode82> >>>>>>> sanitizer_common/sanitizer_**common.h:82: // Debug info routines >>>>>>> On 2012/06/18 18:18:38, dvyukov wrote: >>>>>>> >>>>>>>> Looks like a wrong place for it. It's not quite "common". Can it >>>>>>>> live >>>>>>>> >>>>>>> in >>>>>>> >>>>>>>> symbolizer? >>>>>>>> >>>>>>> >>>>>>> Done. >>>>>>> >>>>>>> >>>>>>> http://codereview.appspot.com/**6303097/diff/1/sanitizer_** >>>>>>> common/sanitizer_symbolizer.cc<http://codereview.appspot.com/6303097/diff/1/sanitizer_common/sanitizer_symbolizer.cc> >>>>>>> File sanitizer_common/sanitizer_**symbolizer.cc (right): >>>>>>> >>>>>>> http://codereview.appspot.com/**6303097/diff/1/sanitizer_** >>>>>>> common/sanitizer_symbolizer.**cc#newcode39<http://codereview.appspot.com/6303097/diff/1/sanitizer_common/sanitizer_symbolizer.cc#newcode39> >>>>>>> sanitizer_common/sanitizer_**symbolizer.cc:39: DWARFSection >>>>>>> debug_info; >>>>>>> On 2012/06/18 18:18:38, dvyukov wrote: >>>>>>> >>>>>>>> You don't need them here. >>>>>>>> You need them only in ctor. >>>>>>>> >>>>>>> >>>>>>> Done. >>>>>>> >>>>>>> >>>>>>> http://codereview.appspot.com/**6303097/diff/1/sanitizer_** >>>>>>> common/sanitizer_symbolizer.**cc#newcode46<http://codereview.appspot.com/6303097/diff/1/sanitizer_common/sanitizer_symbolizer.cc#newcode46> >>>>>>> sanitizer_common/sanitizer_**symbolizer.cc:46: >>>>>>> ReadFileToBuffer(module_name, (char**)&addr, &size, kMaxModuleSize); >>>>>>> On 2012/06/18 18:18:38, dvyukov wrote: >>>>>>> >>>>>>>> That's a bad idea. You need to use mmap(). Otherwise you will soak >>>>>>>> up >>>>>>>> >>>>>>> into >>>>>>> >>>>>>>> memory GBs of uninteresting data. >>>>>>>> >>>>>>> >>>>>>> Done. >>>>>>> >>>>>>> >>>>>>> http://codereview.appspot.com/**6303097/diff/1/sanitizer_** >>>>>>> common/sanitizer_symbolizer.**cc#newcode97<http://codereview.appspot.com/6303097/diff/1/sanitizer_common/sanitizer_symbolizer.cc#newcode97> >>>>>>> sanitizer_common/sanitizer_**symbolizer.cc:97: while >>>>>>> (proc_maps.Next(&start, &end, &offset, module_name, >>>>>>> On 2012/06/18 18:18:38, dvyukov wrote: >>>>>>> >>>>>>>> This ain't gonna work. >>>>>>>> You need 1 DIContext per *module*. >>>>>>>> >>>>>>> Damn, you're right here. Rewrote this part (use loookup by name to >>>>>>> find >>>>>>> a debug info context for a module or create one). >>>>>>> >>>>>>> >>>>>>> This thing will give you N DIContexts per module (code, code, data, >>>>>>>> >>>>>>> data), and >>>>>>> >>>>>>>> you won't find .bss/.tbss sections at all, because they are no >>>>>>>> mapped >>>>>>>> >>>>>>> from the >>>>>>> >>>>>>>> file. So you will have N times memory consumption + will be unable >>>>>>>> to >>>>>>>> >>>>>>> symbolize >>>>>>> >>>>>>>> some data. >>>>>>>> >>>>>>> >>>>>>> Probably I lack understanding here. >>>>>>> OK, what can I do about .bss/.tbss? If they are not mapped into the >>>>>>> memory, then how can I get (virtual) address of data in these >>>>>>> sections >>>>>>> passed to symbolizer? >>>>>> >>>>>> >>>>>> >>>>>> They are mapped, but not from the file. You can't figure out what it >>>>>> is by looking at /proc/self/maps. It's just an anonymous mapping there. >>>>>> In tsan I use dl_iterate_phdr. It provides all the necessary in >>>>>> information (and w/o text parsing). >>>>>> >>>>> >>>>> In ASan we used dl_iterate_phdr as well, but later removed it. >>>>> Probably kcc@ or eugenis@ could comment on the reasons. >>>>> >>>> >>>> It had some bug which I did not manage to understand. >>>> Besides, it does not work on Android. >>>> >>> >>> dl_iterate_phdr works for linux w/o any problems. >>> >> >> So, you suggest to try and use it once again? >> > > I suggest to not keep debug info in memory in 3 or more copies, and to be > able to sumbolize global data. I am OK with anything that will do. > OK, I revived dl_iterate_phdr machinery to collect the list of loaded modules (on Linux). PTAL if you're ok with the current version. http://codereview.appspot.com/6303097/ > > > I can understand if for Android we will have something else, because, >>> well, it *is* platform-dependent. >>> It's /proc/self/maps that does not work. It gives you not the info you >>> are looking for. To some degree it can be fixed, but it won't provide e.g. >>> info about .bss/.tbss. >>> >>> >> -- >> Alexey Samsonov, MSK >> >> > -- Alexey Samsonov, MSK
Sign in to reply to this message.
http://codereview.appspot.com/6303097/diff/16001/asan/asan_stack.cc File asan/asan_stack.cc (right): http://codereview.appspot.com/6303097/diff/16001/asan/asan_stack.cc#newcode62 asan/asan_stack.cc:62: } else if (info.module) { It still prints nothing in some cases. Please print at least pc as it is done below: AsanPrintf(" #%zu 0x%zx\n", frame_num, pc); http://codereview.appspot.com/6303097/diff/16001/sanitizer_common/sanitizer_c... File sanitizer_common/sanitizer_common.h (right): http://codereview.appspot.com/6303097/diff/16001/sanitizer_common/sanitizer_c... sanitizer_common/sanitizer_common.h:69: void MapFileToMemory(const char *file_name, uptr *buff, uptr *buff_size); I think it must be void **buf or better void *MapFileToMemory(const char *file_name, uptr *buff_size); to report failure. http://codereview.appspot.com/6303097/diff/16001/sanitizer_common/sanitizer_l... File sanitizer_common/sanitizer_linux.cc (right): http://codereview.appspot.com/6303097/diff/16001/sanitizer_common/sanitizer_l... sanitizer_common/sanitizer_linux.cc:214: module_name = internal_strdup(info->dlpi_name); Can't info->dlpi_name be NULL? http://codereview.appspot.com/6303097/diff/16001/sanitizer_common/sanitizer_l... sanitizer_common/sanitizer_linux.cc:223: beg = Min(beg, cur_beg); AFAIR, modules are not necessary continuous in memory.
Sign in to reply to this message.
http://codereview.appspot.com/6303097/diff/16001/asan/asan_stack.cc File asan/asan_stack.cc (right): http://codereview.appspot.com/6303097/diff/16001/asan/asan_stack.cc#newcode62 asan/asan_stack.cc:62: } else if (info.module) { On 2012/07/02 09:29:33, dvyukov wrote: > It still prints nothing in some cases. Please print at least pc as it is done Does it? See line 56. > below: > AsanPrintf(" #%zu 0x%zx\n", frame_num, pc); http://codereview.appspot.com/6303097/diff/16001/sanitizer_common/sanitizer_c... File sanitizer_common/sanitizer_common.h (right): http://codereview.appspot.com/6303097/diff/16001/sanitizer_common/sanitizer_c... sanitizer_common/sanitizer_common.h:69: void MapFileToMemory(const char *file_name, uptr *buff, uptr *buff_size); On 2012/07/02 09:29:33, dvyukov wrote: > I think it must be > void **buf > or better > void *MapFileToMemory(const char *file_name, uptr *buff_size); > to report failure. Done. http://codereview.appspot.com/6303097/diff/16001/sanitizer_common/sanitizer_l... File sanitizer_common/sanitizer_linux.cc (right): http://codereview.appspot.com/6303097/diff/16001/sanitizer_common/sanitizer_l... sanitizer_common/sanitizer_linux.cc:214: module_name = internal_strdup(info->dlpi_name); On 2012/07/02 09:29:33, dvyukov wrote: > Can't info->dlpi_name be NULL? Done. http://codereview.appspot.com/6303097/diff/16001/sanitizer_common/sanitizer_l... sanitizer_common/sanitizer_linux.cc:223: beg = Min(beg, cur_beg); On 2012/07/02 09:29:33, dvyukov wrote: > AFAIR, modules are not necessary continuous in memory. Yes, and we're in trouble if two modules can have segments interleaving in memory. Do you know if they can?
Sign in to reply to this message.
http://codereview.appspot.com/6303097/diff/16001/asan/asan_stack.cc File asan/asan_stack.cc (right): http://codereview.appspot.com/6303097/diff/16001/asan/asan_stack.cc#newcode62 asan/asan_stack.cc:62: } else if (info.module) { Ah, OK. On 2012/07/02 10:01:01, samsonov wrote: > On 2012/07/02 09:29:33, dvyukov wrote: > > It still prints nothing in some cases. Please print at least pc as it is done > > Does it? See line 56. > > > below: > > AsanPrintf(" #%zu 0x%zx\n", frame_num, pc); > http://codereview.appspot.com/6303097/diff/16001/sanitizer_common/sanitizer_l... File sanitizer_common/sanitizer_linux.cc (right): http://codereview.appspot.com/6303097/diff/16001/sanitizer_common/sanitizer_l... sanitizer_common/sanitizer_linux.cc:223: beg = Min(beg, cur_beg); On 2012/07/02 10:01:01, samsonov wrote: > On 2012/07/02 09:29:33, dvyukov wrote: > > AFAIR, modules are not necessary continuous in memory. > > Yes, and we're in trouble if two modules can have segments interleaving in > memory. Do you know if they can? I've seen such thing with dl.so. It overlaps with half of other modules. $ cat /proc/self/maps 00400000-0040d000 r-xp 00000000 fc:00 1048699 /bin/cat 0060d000-0060e000 r--p 0000d000 fc:00 1048699 /bin/cat 0060e000-0060f000 rw-p 0000e000 fc:00 1048699 /bin/cat 020f3000-02114000 rw-p 00000000 00:00 0 [heap] 7f5a6c89b000-7f5a6ca15000 r-xp 00000000 fc:00 668767 /lib/libc-2.11.1.so 7f5a6ca15000-7f5a6cc14000 ---p 0017a000 fc:00 668767 /lib/libc-2.11.1.so 7f5a6cc14000-7f5a6cc18000 r--p 00179000 fc:00 668767 /lib/libc-2.11.1.so 7f5a6cc18000-7f5a6cc19000 rw-p 0017d000 fc:00 668767 /lib/libc-2.11.1.so 7f5a6cc19000-7f5a6cc1e000 rw-p 00000000 00:00 0 7f5a6cc1e000-7f5a6cc3e000 r-xp 00000000 fc:00 661626 /lib/ld-2.11.1.so 7f5a6ccb9000-7f5a6ccf8000 r--p 00000000 fc:00 2098188 /usr/lib/locale/en_US.utf8/LC_CTYPE 7f5a6ccf8000-7f5a6ce16000 r--p 00000000 fc:00 2097996 /usr/lib/locale/en_US.utf8/LC_COLLATE 7f5a6ce16000-7f5a6ce19000 rw-p 00000000 00:00 0 7f5a6ce2a000-7f5a6ce2b000 r--p 00000000 fc:00 2098195 /usr/lib/locale/en_US.utf8/LC_NUMERIC 7f5a6ce2b000-7f5a6ce2c000 r--p 00000000 fc:00 2098609 /usr/lib/locale/en_US.utf8/LC_TIME 7f5a6ce2c000-7f5a6ce2d000 r--p 00000000 fc:00 2098614 /usr/lib/locale/en_US.utf8/LC_MONETARY 7f5a6ce2d000-7f5a6ce2e000 r--p 00000000 fc:00 2098619 /usr/lib/locale/en_US.utf8/LC_MESSAGES/SYS_LC_MESSAGES 7f5a6ce2e000-7f5a6ce2f000 r--p 00000000 fc:00 2097749 /usr/lib/locale/en_US.utf8/LC_PAPER 7f5a6ce2f000-7f5a6ce30000 r--p 00000000 fc:00 2097513 /usr/lib/locale/en_US.utf8/LC_NAME 7f5a6ce30000-7f5a6ce31000 r--p 00000000 fc:00 2098629 /usr/lib/locale/en_US.utf8/LC_ADDRESS 7f5a6ce31000-7f5a6ce32000 r--p 00000000 fc:00 2098634 /usr/lib/locale/en_US.utf8/LC_TELEPHONE 7f5a6ce32000-7f5a6ce33000 r--p 00000000 fc:00 2097802 /usr/lib/locale/en_US.utf8/LC_MEASUREMENT 7f5a6ce33000-7f5a6ce3a000 r--s 00000000 fc:00 1720697 /usr/lib/gconv/gconv-modules.cache 7f5a6ce3a000-7f5a6ce3b000 r--p 00000000 fc:00 2098638 /usr/lib/locale/en_US.utf8/LC_IDENTIFICATION 7f5a6ce3b000-7f5a6ce3d000 rw-p 00000000 00:00 0 7f5a6ce3d000-7f5a6ce3e000 r--p 0001f000 fc:00 661626 /lib/ld-2.11.1.so 7f5a6ce3e000-7f5a6ce3f000 rw-p 00020000 fc:00 661626 /lib/ld-2.11.1.so 7f5a6ce3f000-7f5a6ce40000 rw-p 00000000 00:00 0 7fff2582f000-7fff25850000 rw-p 00000000 00:00 0 [stack] 7fff2585c000-7fff2585d000 r-xp 00000000 00:00 0 [vdso] ffffffffff600000-ffffffffff601000 r-xp 00000000 00:00 0 [vsyscall]
Sign in to reply to this message.
On 2012/07/02 10:34:24, dvyukov wrote: > http://codereview.appspot.com/6303097/diff/16001/asan/asan_stack.cc > File asan/asan_stack.cc (right): > > http://codereview.appspot.com/6303097/diff/16001/asan/asan_stack.cc#newcode62 > asan/asan_stack.cc:62: } else if (info.module) { > Ah, OK. > > On 2012/07/02 10:01:01, samsonov wrote: > > On 2012/07/02 09:29:33, dvyukov wrote: > > > It still prints nothing in some cases. Please print at least pc as it is > done > > > > Does it? See line 56. > > > > > below: > > > AsanPrintf(" #%zu 0x%zx\n", frame_num, pc); > > > > http://codereview.appspot.com/6303097/diff/16001/sanitizer_common/sanitizer_l... > File sanitizer_common/sanitizer_linux.cc (right): > > http://codereview.appspot.com/6303097/diff/16001/sanitizer_common/sanitizer_l... > sanitizer_common/sanitizer_linux.cc:223: beg = Min(beg, cur_beg); > On 2012/07/02 10:01:01, samsonov wrote: > > On 2012/07/02 09:29:33, dvyukov wrote: > > > AFAIR, modules are not necessary continuous in memory. > > > > Yes, and we're in trouble if two modules can have segments interleaving in > > memory. Do you know if they can? > > I've seen such thing with dl.so. It overlaps with half of other modules. Yes, we'll have to go hard way here. Updated the patch. > $ cat /proc/self/maps > 00400000-0040d000 r-xp 00000000 fc:00 1048699 > /bin/cat > 0060d000-0060e000 r--p 0000d000 fc:00 1048699 > /bin/cat > 0060e000-0060f000 rw-p 0000e000 fc:00 1048699 > /bin/cat > 020f3000-02114000 rw-p 00000000 00:00 0 [heap] > 7f5a6c89b000-7f5a6ca15000 r-xp 00000000 fc:00 668767 > /lib/libc-2.11.1.so > 7f5a6ca15000-7f5a6cc14000 ---p 0017a000 fc:00 668767 > /lib/libc-2.11.1.so > 7f5a6cc14000-7f5a6cc18000 r--p 00179000 fc:00 668767 > /lib/libc-2.11.1.so > 7f5a6cc18000-7f5a6cc19000 rw-p 0017d000 fc:00 668767 > /lib/libc-2.11.1.so > 7f5a6cc19000-7f5a6cc1e000 rw-p 00000000 00:00 0 > 7f5a6cc1e000-7f5a6cc3e000 r-xp 00000000 fc:00 661626 > /lib/ld-2.11.1.so > 7f5a6ccb9000-7f5a6ccf8000 r--p 00000000 fc:00 2098188 > /usr/lib/locale/en_US.utf8/LC_CTYPE > 7f5a6ccf8000-7f5a6ce16000 r--p 00000000 fc:00 2097996 > /usr/lib/locale/en_US.utf8/LC_COLLATE > 7f5a6ce16000-7f5a6ce19000 rw-p 00000000 00:00 0 > 7f5a6ce2a000-7f5a6ce2b000 r--p 00000000 fc:00 2098195 > /usr/lib/locale/en_US.utf8/LC_NUMERIC > 7f5a6ce2b000-7f5a6ce2c000 r--p 00000000 fc:00 2098609 > /usr/lib/locale/en_US.utf8/LC_TIME > 7f5a6ce2c000-7f5a6ce2d000 r--p 00000000 fc:00 2098614 > /usr/lib/locale/en_US.utf8/LC_MONETARY > 7f5a6ce2d000-7f5a6ce2e000 r--p 00000000 fc:00 2098619 > /usr/lib/locale/en_US.utf8/LC_MESSAGES/SYS_LC_MESSAGES > 7f5a6ce2e000-7f5a6ce2f000 r--p 00000000 fc:00 2097749 > /usr/lib/locale/en_US.utf8/LC_PAPER > 7f5a6ce2f000-7f5a6ce30000 r--p 00000000 fc:00 2097513 > /usr/lib/locale/en_US.utf8/LC_NAME > 7f5a6ce30000-7f5a6ce31000 r--p 00000000 fc:00 2098629 > /usr/lib/locale/en_US.utf8/LC_ADDRESS > 7f5a6ce31000-7f5a6ce32000 r--p 00000000 fc:00 2098634 > /usr/lib/locale/en_US.utf8/LC_TELEPHONE > 7f5a6ce32000-7f5a6ce33000 r--p 00000000 fc:00 2097802 > /usr/lib/locale/en_US.utf8/LC_MEASUREMENT > 7f5a6ce33000-7f5a6ce3a000 r--s 00000000 fc:00 1720697 > /usr/lib/gconv/gconv-modules.cache > 7f5a6ce3a000-7f5a6ce3b000 r--p 00000000 fc:00 2098638 > /usr/lib/locale/en_US.utf8/LC_IDENTIFICATION > 7f5a6ce3b000-7f5a6ce3d000 rw-p 00000000 00:00 0 > 7f5a6ce3d000-7f5a6ce3e000 r--p 0001f000 fc:00 661626 > /lib/ld-2.11.1.so > 7f5a6ce3e000-7f5a6ce3f000 rw-p 00020000 fc:00 661626 > /lib/ld-2.11.1.so > 7f5a6ce3f000-7f5a6ce40000 rw-p 00000000 00:00 0 > 7fff2582f000-7fff25850000 rw-p 00000000 00:00 0 [stack] > 7fff2585c000-7fff2585d000 r-xp 00000000 00:00 0 [vdso] > ffffffffff600000-ffffffffff601000 r-xp 00000000 00:00 0 > [vsyscall]
Sign in to reply to this message.
On 2012/07/02 14:39:24, samsonov wrote: > On 2012/07/02 10:34:24, dvyukov wrote: > > http://codereview.appspot.com/6303097/diff/16001/asan/asan_stack.cc > > File asan/asan_stack.cc (right): > > > > http://codereview.appspot.com/6303097/diff/16001/asan/asan_stack.cc#newcode62 > > asan/asan_stack.cc:62: } else if (info.module) { > > Ah, OK. > > > > On 2012/07/02 10:01:01, samsonov wrote: > > > On 2012/07/02 09:29:33, dvyukov wrote: > > > > It still prints nothing in some cases. Please print at least pc as it is > > done > > > > > > Does it? See line 56. > > > > > > > below: > > > > AsanPrintf(" #%zu 0x%zx\n", frame_num, pc); > > > > > > > > http://codereview.appspot.com/6303097/diff/16001/sanitizer_common/sanitizer_l... > > File sanitizer_common/sanitizer_linux.cc (right): > > > > > http://codereview.appspot.com/6303097/diff/16001/sanitizer_common/sanitizer_l... > > sanitizer_common/sanitizer_linux.cc:223: beg = Min(beg, cur_beg); > > On 2012/07/02 10:01:01, samsonov wrote: > > > On 2012/07/02 09:29:33, dvyukov wrote: > > > > AFAIR, modules are not necessary continuous in memory. > > > > > > Yes, and we're in trouble if two modules can have segments interleaving in > > > memory. Do you know if they can? > > > > I've seen such thing with dl.so. It overlaps with half of other modules. > > Yes, we'll have to go hard way here. Updated the patch. LGTM > > > $ cat /proc/self/maps > > 00400000-0040d000 r-xp 00000000 fc:00 1048699 > > /bin/cat > > 0060d000-0060e000 r--p 0000d000 fc:00 1048699 > > /bin/cat > > 0060e000-0060f000 rw-p 0000e000 fc:00 1048699 > > /bin/cat > > 020f3000-02114000 rw-p 00000000 00:00 0 > [heap] > > 7f5a6c89b000-7f5a6ca15000 r-xp 00000000 fc:00 668767 > > /lib/libc-2.11.1.so > > 7f5a6ca15000-7f5a6cc14000 ---p 0017a000 fc:00 668767 > > /lib/libc-2.11.1.so > > 7f5a6cc14000-7f5a6cc18000 r--p 00179000 fc:00 668767 > > /lib/libc-2.11.1.so > > 7f5a6cc18000-7f5a6cc19000 rw-p 0017d000 fc:00 668767 > > /lib/libc-2.11.1.so > > 7f5a6cc19000-7f5a6cc1e000 rw-p 00000000 00:00 0 > > 7f5a6cc1e000-7f5a6cc3e000 r-xp 00000000 fc:00 661626 > > /lib/ld-2.11.1.so > > 7f5a6ccb9000-7f5a6ccf8000 r--p 00000000 fc:00 2098188 > > /usr/lib/locale/en_US.utf8/LC_CTYPE > > 7f5a6ccf8000-7f5a6ce16000 r--p 00000000 fc:00 2097996 > > /usr/lib/locale/en_US.utf8/LC_COLLATE > > 7f5a6ce16000-7f5a6ce19000 rw-p 00000000 00:00 0 > > 7f5a6ce2a000-7f5a6ce2b000 r--p 00000000 fc:00 2098195 > > /usr/lib/locale/en_US.utf8/LC_NUMERIC > > 7f5a6ce2b000-7f5a6ce2c000 r--p 00000000 fc:00 2098609 > > /usr/lib/locale/en_US.utf8/LC_TIME > > 7f5a6ce2c000-7f5a6ce2d000 r--p 00000000 fc:00 2098614 > > /usr/lib/locale/en_US.utf8/LC_MONETARY > > 7f5a6ce2d000-7f5a6ce2e000 r--p 00000000 fc:00 2098619 > > /usr/lib/locale/en_US.utf8/LC_MESSAGES/SYS_LC_MESSAGES > > 7f5a6ce2e000-7f5a6ce2f000 r--p 00000000 fc:00 2097749 > > /usr/lib/locale/en_US.utf8/LC_PAPER > > 7f5a6ce2f000-7f5a6ce30000 r--p 00000000 fc:00 2097513 > > /usr/lib/locale/en_US.utf8/LC_NAME > > 7f5a6ce30000-7f5a6ce31000 r--p 00000000 fc:00 2098629 > > /usr/lib/locale/en_US.utf8/LC_ADDRESS > > 7f5a6ce31000-7f5a6ce32000 r--p 00000000 fc:00 2098634 > > /usr/lib/locale/en_US.utf8/LC_TELEPHONE > > 7f5a6ce32000-7f5a6ce33000 r--p 00000000 fc:00 2097802 > > /usr/lib/locale/en_US.utf8/LC_MEASUREMENT > > 7f5a6ce33000-7f5a6ce3a000 r--s 00000000 fc:00 1720697 > > /usr/lib/gconv/gconv-modules.cache > > 7f5a6ce3a000-7f5a6ce3b000 r--p 00000000 fc:00 2098638 > > /usr/lib/locale/en_US.utf8/LC_IDENTIFICATION > > 7f5a6ce3b000-7f5a6ce3d000 rw-p 00000000 00:00 0 > > 7f5a6ce3d000-7f5a6ce3e000 r--p 0001f000 fc:00 661626 > > /lib/ld-2.11.1.so > > 7f5a6ce3e000-7f5a6ce3f000 rw-p 00020000 fc:00 661626 > > /lib/ld-2.11.1.so > > 7f5a6ce3f000-7f5a6ce40000 rw-p 00000000 00:00 0 > > 7fff2582f000-7fff25850000 rw-p 00000000 00:00 0 > [stack] > > 7fff2585c000-7fff2585d000 r-xp 00000000 00:00 0 > [vdso] > > ffffffffff600000-ffffffffff601000 r-xp 00000000 00:00 0 > > [vsyscall]
Sign in to reply to this message.
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
