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
