Non-SFI NaCl: Do not create unwind tables for libc_free.c

Non-SFI NaCl: Do not create unwind tables for libc_free.c

ARM GCC emits references to EABI symbols like __aeabi_unwind_cpp_pr0
and they are causing a link error because recently we have added
-Wl,--no-undefined.

TEST=./out/Release/browser_tests --gtest_filter='*NonSfi*'
     # Note we cannot test this on ARM yet.
TEST=ninja browser_tests (for ARM Chrome OS)
BUG=None

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=284807

[hamaji, 2014-07-21 06:36:05]

I guess this is not failing on ARM builders because they are not building
browser_tests.

[Mark Seaborn, 2014-07-21 22:46:07]

On 20 July 2014 23:36, <hamaji@chromium.org> wrote:

> Reviewers: Mark Seaborn,
>
> Message:
> I guess this is not failing on ARM builders because they are not building
> browser_tests.


Are you referring to the ARM Chrome OS builders?

The ARM Chromium builders (non-Chrome-OS) are currently broken and failing
the "compile" step with:
arm-linux-gnueabihf-gcc: error: find-bad-constructs: No such file or
directory
(e.g. see
http://build.chromium.org/p/chromium.fyi/builders/Linux%20ARM%20Cross-Compile...
)

I've reported that as
https://code.google.com/p/chromium/issues/detail?id=395832.

Cheers,
Mark

To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[Mark Seaborn, 2014-07-21 22:49:33]

https://codereview.chromium.org/401243002/diff/1/chrome/test/data/nacl/nonsfi...
File chrome/test/data/nacl/nonsfi/libc_free.c (right):

https://codereview.chromium.org/401243002/diff/1/chrome/test/data/nacl/nonsfi...
chrome/test/data/nacl/nonsfi/libc_free.c:214: // ARM GCC emits references to
them. They should not be called unless
Hmm, where in the code do the references come from?

https://codereview.chromium.org/401243002/diff/1/chrome/test/data/nacl/nonsfi...
chrome/test/data/nacl/nonsfi/libc_free.c:217: void __aeabi_unwind_cpp_pr0() {
__libnacl_irt_basic.exit(kUnwindExitCode); }
Nit: the function body should be on a separate line.

Just calling __builtin_trap() to exit would also be simpler.

[hamaji, 2014-07-22 08:17:23]

https://codereview.chromium.org/401243002/diff/1/chrome/test/data/nacl/nonsfi...
File chrome/test/data/nacl/nonsfi/libc_free.c (right):

https://codereview.chromium.org/401243002/diff/1/chrome/test/data/nacl/nonsfi...
chrome/test/data/nacl/nonsfi/libc_free.c:214: // ARM GCC emits references to
them. They should not be called unless
On 2014/07/21 22:49:33, Mark Seaborn wrote:
> Hmm, where in the code do the references come from?

They are in exidx section. Ah, -funwind-tables is set and dropping this flag
seems to be a better solution.

https://codereview.chromium.org/401243002/diff/1/chrome/test/data/nacl/nonsfi...
chrome/test/data/nacl/nonsfi/libc_free.c:217: void __aeabi_unwind_cpp_pr0() {
__libnacl_irt_basic.exit(kUnwindExitCode); }
On 2014/07/21 22:49:33, Mark Seaborn wrote:
> Nit: the function body should be on a separate line.
> 
> Just calling __builtin_trap() to exit would also be simpler.

It seems ARM GCC uses abort() to implement __builtin_trap, but anyway, this code
was removed.

[Mark Seaborn, 2014-07-22 15:19:48]

LGTM

https://codereview.chromium.org/401243002/diff/1/chrome/test/data/nacl/nonsfi...
File chrome/test/data/nacl/nonsfi/libc_free.c (right):

https://codereview.chromium.org/401243002/diff/1/chrome/test/data/nacl/nonsfi...
chrome/test/data/nacl/nonsfi/libc_free.c:214: // ARM GCC emits references to
them. They should not be called unless
On 2014/07/22 08:17:23, hamaji wrote:
> On 2014/07/21 22:49:33, Mark Seaborn wrote:
> > Hmm, where in the code do the references come from?
> 
> They are in exidx section. Ah, -funwind-tables is set and dropping this flag
> seems to be a better solution.

Yes, that's what we do for nacl_helper_bootstrap, which also avoids using
libc/libgcc.

[Sam Clegg, 2014-07-22 22:20:37]

On 2014/07/22 15:19:48, Mark Seaborn wrote:
> LGTM
> 
>
https://codereview.chromium.org/401243002/diff/1/chrome/test/data/nacl/nonsfi...
> File chrome/test/data/nacl/nonsfi/libc_free.c (right):
> 
>
https://codereview.chromium.org/401243002/diff/1/chrome/test/data/nacl/nonsfi...
> chrome/test/data/nacl/nonsfi/libc_free.c:214: // ARM GCC emits references to
> them. They should not be called unless
> On 2014/07/22 08:17:23, hamaji wrote:
> > On 2014/07/21 22:49:33, Mark Seaborn wrote:
> > > Hmm, where in the code do the references come from?
> > 
> > They are in exidx section. Ah, -funwind-tables is set and dropping this flag
> > seems to be a better solution.
> 
> Yes, that's what we do for nacl_helper_bootstrap, which also avoids using
> libc/libgcc.

Can we land this ASAP, as I think its blocking this change:
https://codereview.chromium.org/411633003/

[Sam Clegg, 2014-07-22 22:44:20]

The CQ bit was checked by sbc@chromium.org

[commit-bot: I haz the power, 2014-07-22 22:46:27]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/hamaji@chromium.org/401243002/20001

[commit-bot: I haz the power, 2014-07-22 23:37:44]

Change committed as 284807