the-tcpdump-group / libpcap

the LIBpcap interface to various kernel packet capture mechanism
https://www.tcpdump.org/
Other
2.72k stars 855 forks source link

Add thread_local macro for HP-UX #1349

Closed michael-o closed 2 months ago

michael-o commented 2 months ago

Compiles like a charm after that:

# file ./libpcap.so.1.10.5
./libpcap.so.1.10.5:    ELF-32 shared object file - IA64
guyharris commented 2 months ago

Is that "for HP-UX" or "for HP aCC"? If it's really the latter, you should test for __HP_aCC instead of testing for __hpux.

michael-o commented 2 months ago

Is that "for HP-UX" or "for HP aCC"? If it's really the latter, you should test for __HP_aCC instead of testing for __hpux.

Technically you are right, the symbol has been introduced in 10.30, but for the past 10 or more years the only compiler is aCC. GCC stuck with 4.2.x and died. Both macros are synonymous for me in this case. If you want it to be precise, I can change the PR for sure.

infrastation commented 2 months ago

Connect makes a very similar change in their package.

michael-o commented 2 months ago

Connect makes a very similar change in their package.

I intentionally don't use their packages for a lot of reasons, but the biggest one is that they don't upstream them.

guyharris commented 2 months ago

I intentionally don't use their packages for a lot of reasons, but the biggest one is that they don't upstream them.

I think Denis is recommending that we upstream their changes ourselves when they make sense, not that you use their packages instead of our source code.

infrastation commented 2 months ago

I managed to contact Connect maintainer several days ago and I hope they will help upstreaming as many of their changes as possible and take the credit where credit is due (if they wish).

That said, the Connect package patch currently uses a different approach to this particular problem:

--- libpcap-1.10.5.orig/thread-local.h  2024-08-30 20:34:14.000000000 +0100
+++ libpcap-1.10.5.patched/thread-local.h       2024-09-01 11:52:10.000000000 +0100
@@ -64,6 +64,8 @@
          defined __SUNPRO_C || \
          defined __xlC__
     #define thread_local __thread
+  #elif defined __hpux
+     #define thread_local
   #else
     #error "Cannot define thread_local"
   #endif

Hinging this on __HP_aCC seems a better style. Also if thread-local storage requires a specific minimum version of HP C (which would explain the difference in solutions), then PCAP_IS_AT_LEAST_HP_C_VERSION() would seem a better match. I do not have enough information to tell reliably though.

Finally, changes such as this should include the OS and the compiler versions, as well as the compiler warning/error in the commit message.

michael-o commented 2 months ago

Let me check the Compiler manual again on Monday, but I do remember 10.30 which is ages old.

guyharris commented 2 months ago

if thread-local storage requires a specific minimum version of HP C

If that's the case, then not only should we use PCAP_IS_AT_LEAST_HP_C_VERSION() to test for those versions, but, for the versions of aCC that don't support it, we should do for those versions what we do for Tiny C, and report a warning that some routines will not be thread-safe.

michael-o commented 2 months ago

The latest aCC is A.06.29 from 2016. I had a lengthy talk with HPE regarding a new version. Upshot: there won't be a newer version. They will fix bugs at most. I consider this keyword to be supported for the past 15+ years.

guyharris commented 2 months ago

Upshot: there won't be a newer version.

HPE's hardware future, for now, is x86-64, not Itanium or PA-RISC (given that both of those are dead), and its operating system future is Linux (with the ability to run Itanium HP-UX binaries via binary-to-binary translation and, presumably, mapping of either system calls or library calls). As such, I don't see HPE spending a lot of energy on a platform with its life clock ticking away.

I consider this keyword to be supported for the past 15+ years.

So HP's C compilers, dating back to 2009, all support __thread?

Note that compilers that don't support enough of C99 to allow libpcap to be compiled are not of interest - I don't know when HP first supported that - so it might not be necessary to worry about compilers that old.

michael-o commented 2 months ago

man 3 pthread on 11.31 says:

  The HP-UX compiler supports a thread local storage (TLS) storage
  class.  (This is not a POSIX standard feature.) TLS is identical to
  TSD, except functions are not required to create/set/get values.  TLS
  variables are accessed just like normal global variables.  TLS
  variables can be declared using the following syntax:

        __thread     int     zyx;

  The keyword __thread tells the compiler that zyx is a TLS variable.
  Now each thread can set or get TLS with statements such as:

       zyx = 21;

  Each thread will have a different value associated with zyx.

  TLS variables can be statically initialized.  Uninitialized TLS
  variables will be set to zero.  Dynamically loaded libraries (with
  shl_load()) can declare and use TLS variables.

....

man 3 pthread on 11.11 (HP-UX hp7dw B.11.11 U 9000/785 2014240576 unlimited-user license) says:

  The HP-UX compiler supports a thread local storage (TLS) storage
  class.  (This is not a POSIX standard feature.) TLS is identical to
  TSD, except functions are not required to create/set/get values.  TLS
  variables are accessed just like normal global variables.  TLS
  variables can be declared using the following syntax:

        __thread     int     zyx;

  The keyword __thread tells the compiler that zyx is a TLS variable.
  Now each thread can set or get TLS with statements such as:

       zyx = 21;

  Each thread will have a different value associated with zyx.

  TLS variables cannot be statically initialized, all are initially
  zero.  Dynamically loaded libraries (via shl_load()) cannot declare
  (but may use) TLS variables.

The same.

HP C/HP-UX Reference Manual for HP 9000 Computers from 2001-11-13 says:

__thread 
Beginning with the HP-UX 10.30 operating system release, this 
HP-specific keyword defines a thread specific data variable, 
distinguishing it from other data items that are shared by all threads. 
With a thread-specific data variable, each thread has its own copy of the 
data item. These variables eliminate the need to allocate thread-specific 
data dynamically, thus improving performance.
This keyword is implemented as an HP-specific type qualifier, with the 
same syntax as const and volatile, but not the same semantics. 
Syntax examples:
__thread int j=2; 
int main()
{
   j = 20;
}
Semantics for the __thread keyword: Only variables of static duration 
can be thread specific. Thread specific data objects can not be initialized. 
Pointers of static duration that are not thread specific may not be 
initialized with the address of a thread specific object — assignment is 
okay. All global variables, thread specific or not, are initialized to zero by 
the linker implicitly.

That's the sources I have. From my understanding this has been working for ages unless someone proves otherwise.

michael-o commented 2 months ago

Upshot: there won't be a newer version.

HPE's hardware future, for now, is x86-64, not Itanium or PA-RISC (given that both of those are dead), and its operating system future is Linux (with the ability to run Itanium HP-UX binaries via binary-to-binary translation and, presumably, mapping of either system calls or library calls). As such, I don't see HPE spending a lot of energy on a platform with its life clock ticking away.

These rumors have never been confirmed nor did HPE approach to customers (us) that something like Aries 2 is in the works. There is not migration path. There is just a hard cut. I have also a lengthly talk with the regarding a newer compiler and the admitted that they failed to port LLVM to HP-UX when it was the time for.

I consider this keyword to be supported for the past 15+ years.

So HP's C compilers, dating back to 2009, all support __thread?

Note that compilers that don't support enough of C99 to allow libpcap to be compiled are not of interest - I don't know when HP first supported that - so it might not be necessary to worry about compilers that old.

See my lengthly answer.

michael-o commented 2 months ago

Any objections to merge this after the information I have provided?

infrastation commented 2 months ago

Guy has the most experience with HP-UX, so what looks good to him should be the best approach.