Re: [RFC] Add C11 threads.h support.

[Juan Manuel Torres Palma <j dot m dot torrespalma at gmail dot com>]

I get all of your points now, it's unbelievable how much I'm learning
during the process, thanks for your guidance.

After running the testsuite I'm getting error regarding the
linknamespace test and conform test. Conform tests errors don't make
me worry that much because they seem to appear on other headers too
and are being ignored. However, in linknamespace test appear a lot of
new functions that I didn't even think about that seem to be in user's
namespace.

I attach my threads.h-data, linknamespace.out and conform.out for some feedback.

Cheers.

2015-06-29 16:30 GMT+02:00 Joseph Myers <joseph@codesourcery.com>:
> On Sat, 27 Jun 2015, Juan Manuel Torres Palma wrote:
>
>> > I'm not sure it's useful without any targets that would define the types
>> > differently; I think bits/ headers are only needed where there are such
>> > target-dependencies (so bits/thread-shared-types.h is needed, but maybe
>> > not bits/threadstypes.h).
>>
>> I have moved it to nptl/threadstypes.h now.
>
> I don't think a separate header is needed at all.  Just define the types
> directly in threads.h unless either (a) different definitions are needed
> on different systems or (b) the same type definitions need sharing between
> more than one public header.  If either of those becomes true, put the
> relevant definitions in a bits/ header (any non-public headers - headers
> users should not include directly - should be in bits/).
>
>> Works well, but it fails. I guess this is what I should expect because
>> the generated ABI with new symbols doesn't match with the ABI defined
>> in libpthread.abilist. However I get a symbol that I'm not really sure
>> of. It's _nl_default_dirname, that was originally defined to 0x12 and
>> after building it's defined to 0x43. Reading ChangeLog I couldn't
>> really get an idea of what this symbol is for.
>
> This indicates you didn't configure with --prefix=/usr.  Configuring with
> a different prefix breaks the ABI (see bug 14664).  So always configure
> --prefix=/usr (and then set install_root=/some/where to install under
> /some/where when installing).
>
>> > Each function should have a comment explaining its semantics in the
>> > header.  Argument names need to be in the implementation namespace (the
>> > conform/ tests will tell you if you get this wrong, once you've added
>> > threads.h data to them).  Stray space before ')'.
>>
>> I have done this too, but I don't understand why argument names need
>> to be preceded of "__" in this case. I have seen it in other header
>> files, but why? I thought the assembler only cares about the types
>> defined in the function prototype.
>
> Because "thr", for example, is in the user's namespace, the user can
> legitimately do
>
> #define thr ** break this **
> #include <threads.h>
>
> which would break if "thr" is used as an argument name in threads.h.  So
> installed headers need to be namespace-clean in terms of the set of
> identifiers appearing as preprocessing tokens in the header - regardless
> of the syntactic position in which those preprocessing tokens appear.
>
>> > You need to call the implementation-namespace names such as
>> > __pthread_once.  It's not enough that they exist if you then use the other
>> > names for those functions; you must avoid any strong references (defined
>> > or undefined) to names outside the C11 and implementation namespaces (and
>> > any uses of weak undefined symbols outside those namespaces that might get
>> > executed for conforming code).
>>
>> I don't get this point either. I thought this code will be executing
>> inside the library, where __pthread_* symbols are exactly the same
>> than pthread_* symbols. So, if the user redefines pthread_* symbols
>> (He's a llowed to do that), won't affect symbols inside library. Am I
>> wrong?
>
> If you call a function in the user's namespace, it's a problem (a) for
> static linking, (b) for calls from one shared library to another and (c)
> for calls within a shared library if they go via the PLT.  (In the last
> case, for most functions you'd also get a failure from elf/check-localplt
> if you accidentally introduce new PLT references, but the hidden_proto /
> hidden_def mechanism can be used to avoid PLT references, if the call
> would otherwise be OK for static linking.  I take it you ran the full
> glibc testsuite before and after your patch and made sure there were no
> new failures?  In the case of (a), adding threads.h to the conform/ tests
> will ensure the testsuite checks the namespace for you.)
>
> --
> Joseph S. Myers
> joseph@codesourcery.com



-- 
Juan Manuel Torres Palma.
Computer Science Student at Universidad de Granada.

Attachment: threads.h-data
Description: Binary data

Attachment: conform.out
Description: Binary data

Attachment: linknamespace.out
Description: Binary data