Committed upstream:
http://llvm.org/viewvc/llvm-project?view=revision&revision=196480

On Thu, Dec 5, 2013 at 11:39 AM, Konstantin Serebryany
<konstantin.s.serebry...@gmail.com> wrote:
> On Wed, Dec 4, 2013 at 6:16 PM, Jakub Jelinek <ja...@redhat.com> wrote:
>> On Wed, Dec 04, 2013 at 06:09:56PM +0400, Konstantin Serebryany wrote:
>>> This is a maintenance problem because we can not test if we broke
>>> something during development.
>>> e.g. clang doesn't seem to support -fno-dwarf2-cfi-asm
>>
>> It does, at least both clang 3.3 (from Fedora 19) and clang
>> 3.4 r194685 (which I've built myself some time ago just to look at the
>> use-after-return etc. sanitization).
>
> That's not what I see in my build:
> % cat asm_test.cc
> void foo() {
>    __asm__ __volatile__(".cfi_adjust_cfa_offset 100");
> }
> % clang -c asm_test.cc -fno-dwarf2-cfi-asm
> % clang -c asm_test.cc
> % gcc -c asm_test.cc
> % gcc -c asm_test.cc -fno-dwarf2-cfi-asm
> asm_test.cc: Assembler messages:
> asm_test.cc:2: Error: CFI instruction used without previous .cfi_startproc
> %
>
> Probably one needs to configure clang in some special way (e.g. to use
> external as?).
>
> Anyway, I've sent this for review: http://llvm-reviews.chandlerc.com/D2336
> and tested it like this:
>
> % clang++ -I. -c tsan/rtl/*.cc tsan/rtl/*.S -fno-dwarf2-cfi-asm
> % clang++ -I. -c tsan/rtl/*.cc tsan/rtl/*.S
> % g++ -I. -c tsan/rtl/*.cc tsan/rtl/*.S
> tsan/rtl/tsan_symbolize_addr2line_linux.cc: In function ‘void
> __tsan::InitModule(__tsan::ModuleDesc*)’:
> tsan/rtl/tsan_symbolize_addr2line_linux.cc:73:77: warning: missing
> sentinel in function call [-Wformat]
> % g++ -I. -c tsan/rtl/*.cc tsan/rtl/*.S -fno-dwarf2-cfi-asm
> tsan/rtl/tsan_symbolize_addr2line_linux.cc: In function ‘void
> __tsan::InitModule(__tsan::ModuleDesc*)’:
> tsan/rtl/tsan_symbolize_addr2line_linux.cc:73:77: warning: missing
> sentinel in function call [-Wformat]
> %
>
> (I don't get the gcc warning, but that's unrelated).
>
> I can not test the change in tsan/rtl/tsan_rtl_amd64.S properly
> because I could not make it fail w/o the change, even with
> -fno-dwarf2-cfi-asm
> But looks correct.
>
>>
>>> I can commit a change similar to your cfi-related changes
>>> (guarded by SANITIZER_DONT_USE_CFI_ASM instead of
>>> __GCC_HAVE_DWARF2_CFI_ASM), but the problem will arise again
>>
>> Why?  Is it so hard to remember that when you add .cfi_* directives
>> they should be guarded by that macro?  Even if the patch author
>> forgets about that, patch reviewer should catch that.
>
> Yes, there is a good chance to catch this during review, but not 100%.
> And cfi is not the only problem like this.
>
> --kcc
>
>
>>
>>         Jakub

Reply via email to