#21925 closed defect (fixed)

Tor Browser based on ESR52 can't get built with ASan and FORTIFY_SOURCE

Reported by: gk Owned by: tbb-team
Priority: Medium Milestone:
Component: Applications/Tor Browser Version:
Severity: Normal Keywords: GeorgKoppen201712, TorBrowserTeam201712R
Cc: Actual Points:
Parent ID: #21998 Points:
Reviewer: Sponsor:

Description

Similarly to #17508 ASan seems to be unhappy with FORTIFY_SOURCE when using .mozconfig-asan:

In file included from /usr/include/features.h:356:0,
                 from /usr/include/limits.h:27,
                 from /home/debian/install/gcc/lib/gcc/x86_64-unknown-linux-gnu/5.4.0/include-fixed/limits.h:168,
                 from /home/debian/install/gcc/lib/gcc/x86_64-unknown-linux-gnu/5.4.0/include-fixed/syslimits.h:7,
                 from /home/debian/install/gcc/lib/gcc/x86_64-unknown-linux-gnu/5.4.0/include-fixed/limits.h:34,
                 from /home/debian/build/tor-browser/obj-x86_64-pc-linux-gnu/js/src/ctypes/libffi/include/ffi.h:76,
                 from /home/debian/build/tor-browser/js/src/ctypes/libffi/src/x86/ffi64.c:30:
/home/debian/build/tor-browser/js/src/ctypes/libffi/src/x86/ffi64.c: In function 'ffi_call':
/usr/include/x86_64-linux-gnu/bits/string3.h:49:1: error: inlining failed in call to always_inline 'memcpy': function attribute mismatch
 __NTH (memcpy (void *__restrict __dest, __const void *__restrict __src,
 ^
/home/debian/build/tor-browser/js/src/ctypes/libffi/src/x86/ffi64.c:485:4: error: called from here
    memcpy (argp, avalue[i], size);
    ^

Child Tickets

Change History (9)

comment:1 Changed 11 months ago by boklm

Parent ID: #21998

Reverting the patch from https://bugzilla.mozilla.org/show_bug.cgi?id=1279096 is fixing the build issue.

comment:2 in reply to:  1 Changed 11 months ago by gk

Replying to boklm:

Reverting the patch from https://bugzilla.mozilla.org/show_bug.cgi?id=1279096 is fixing the build issue.

I guess that's because the ASan that is shipped with GCC 5.4.0 is old enough to not hit this Mozilla bug. Assuming this is indeed an LLVM 3.8 issue then we can see when this would hit compiling with GCC

LLVM 3.7 rev 246985
LLVM 3.8 rev 271801

GCC 5.4.0 rev 221802
GCC 6.4.0 rev 253555
GCC 7.2.0 rev 285547

So, when switching to GCC 6.4.0 we might still be good. But as soon as we use GCC 7.x that would not be the case anymore. Anyway, now that we know what is causing it we should probably file a Mozilla bug to get the GCC bits right.

Last edited 11 months ago by gk (previous) (diff)

comment:3 Changed 11 months ago by gk

I still hit that one with GCC 6.4.0 and have filed https://bugzilla.mozilla.org/show_bug.cgi?id=1422254.

comment:4 Changed 11 months ago by tom

You're not supposed to build ASAN with FORTIFY, or so I am told by people who I think know what they're talking about.

See:
https://bugzilla.mozilla.org/show_bug.cgi?id=1377553
https://bugzilla.mozilla.org/show_bug.cgi?id=1419607
https://bugzilla.mozilla.org/show_bug.cgi?id=1418052

comment:5 in reply to:  4 Changed 11 months ago by gk

Replying to tom:

You're not supposed to build ASAN with FORTIFY, or so I am told by people who I think know what they're talking about.

See:
https://bugzilla.mozilla.org/show_bug.cgi?id=1377553
https://bugzilla.mozilla.org/show_bug.cgi?id=1419607
https://bugzilla.mozilla.org/show_bug.cgi?id=1418052

Which does not mean compilation should break. Apart from that there is still an issue open on the ASan side (https://github.com/google/sanitizers/issues/247) and glibc side to fix that: https://sourceware.org/bugzilla/show_bug.cgi?id=20422 (there seem to be some additional benefits of FORTIFY_SOURCE mentioned in this icket). So, it's not that one is generally not supposed to build with with ASan and FORTIFY_SOURCE: they just don't work together right now.

Now, this ticket got filed when we still shipped the hardened series. While those were not builds meant to be used in production mode they were more production-like than the ASan builds we currently envision: those with --enable-debug and --enable-tests etc. and not distributed to users. I guess one could argue for those builds at least it is fine to disable FORTIFIY_SOURCE?

comment:6 Changed 11 months ago by gk

Keywords: TorBrowserTeam201712R added
Status: newneeds_review

I made a patch for disabling FORTIFY_SOURCE for now. It's in bug_21925_v2 (https://gitweb.torproject.org/user/gk/tor-browser.git/commit/?h=bug_21925_v2&id=1b16d85c3fb36565a8f36bb7c9e4a5890b65c143) in my public tor-browser repo.

comment:7 Changed 11 months ago by boklm

This patch looks good to me. I started a build to confirm that it is fixing the build issue.

comment:8 Changed 11 months ago by boklm

And I can confirm this is fixing the build issue for me.

comment:9 Changed 11 months ago by gk

Keywords: GeorgKoppen201712 added
Resolution: fixed
Status: needs_reviewclosed

Thanks, merged to tor-browser-52.5.2esr-7.5-1 as commit 1b16d85c3fb36565a8f36bb7c9e4a5890b65c143.

Note: See TracTickets for help on using tickets.