The patch looks pretty good as far as I can see. That said, I've been out of the loop for quite a while, so do take my approval with a grain of salt.

In D42748#1510087, @MaskRay wrote:

@ed I want to knore more about your motivation to add --export-dynamic to the condition in D29982. Why do you need .dynamic in a position dependent executable for CloudABI, which has no shared object dependency on the linker command line?

Thanks for working on this!

In D42748#994676, @arichardson wrote:

The dynamic symbol table is still included, it's only the PT_DYNAMIC, .dynamic and the _DYNAMIC symbol that are excluded.

This effectively reverts rL295240, right? If so, I'd personally oppose to this change.

In D41500#965157, @zhmu wrote:

@ed I tried to keep things as much in line as the other <os>::linker::Construct() functions do. Do you wish to stray from that path?

I actually prefer to keep it like this as it's quite readable already, but YMMV of course :-)

Hey! Sorry for the breakage. I thought I did a good job of testing this change on various operating systems, but somehow this still fell through the cracks. Odd.

Final call: do any of you have any more feedback on this change? If not, I will go ahead and commit this change one of these days (together with D32937).

Hi Davide,

These changes look all right to me, except that I'd like it if someone else could take a look at Ananas.cpp as well. Is anyone interested in taking a look?

This change looks good to me. Does anyone mind if I were to commit this?

Worth mentioning: the latest version of macOS now supports clock_gettime(). Maybe better to leave the code as is and simply axe the Mach time code at some point in the future?

In D30681#694312, @keith.walker.arm wrote:

This change looks OK to me.

Any more feedback before I commit this change to the trunk?

Restore comment that was lost during sync to latest code.

Remove numbers from llvm-readobj output to make test future proof.

Sync to latest codebase.

Friendly ping! :-)

I'd be interested in seeing a feature like this appearing. Any chance this feature may be part of Clang 4.0?

Also an interesting observation: _LIBUNWIND_IS_BAREMETAL seems to be used only in this single source file and only provides an implementation for ARMv6.

Add a comment after the 'else' to indicate this code applies to ARM.

Interesting observation: dwarf_index_section_length stores the length of the section, whereas arm_section_length stores the number of entries in the section. This feels a bit unnatural in my opinion. Should this be tackled in this change as well, or do we want to do this separately?

I'm the author of FreeBSD's utmpx. LGTM!

Not that it helps much, but with this patch I can at least get all of the packages for CloudABI to build on ARMv6. I can't test whether the resulting binaries work yet, as I'm at a conference. I will be able to test this for you on Monday.

In D22954#512472, @ed wrote:

I just experienced this error while trying to build glib for i686.

I just experienced this error while trying to build glib for i686.

Friendly ping. :-)

Seems to fix the issue. Thanks!

I've just built a copy of LLVM and Clang with these patches applied. With that compiler I did a full rebuild of all core CloudABI libraries, including the C library's unit test suite (~1000 tests), with -fstack-protector-all and SafeStack enabled. All unit tests still seem to pass.

Thanks for working on this! I know too little of LLVM internals to review this change properly, but I'd be more than happy to run our C library test suite against this. Could you also send out the patch for Clang you mentioned, so I can test this?

Okay. Now that my other change has landed, I think we can go ahead with this change as well. Any thoughts?

Put tls-initial-exec-local.s back in now that the GOT works properly.