Re: [PATCH v2 2/2] This patch adds support to debug thread local variables defined in shared libraries in AIX.

[Aditya Kamath <Aditya.Kamath1@ibm.com>]

[-- Attachment #1: Type: text/plain, Size: 2815 bytes --]

Hi Ulrich and community members,

Thank you for your feedback so far. I am sending a v3 patch of the same addressing your concerns soon.

>If you have two dynamic libraries, both defining a single
>TLS symbol, they will both get offset 0.

This is right. I get why you Said this. That is why I decided to have two strategies of which in strategy 2 of the new patch we match R_TLSM by symbol name, i.e. by reading the loader symbol name from the R_TLSM reloc entry and check whether the library we are looking for exports that name. Symbol names are unique under dynamic linker search-order rules, so the first match correctly identifies which library the slot belongs to.

>On the positive side, this means you can do the whole reloc
>scanning fully in the fetch_tls_load_module_address hook
>(since you only need the objfile - the offset doesn't help
>you anything anyway), and simply return the module ID from
>that hook.

rs6000_aix_fetch_tls_load_module_address () now does exactly this where it calls rs6000_aix_find_module_id(objfile) which scans the .loader section using only the objfile and returns the module-id. The offset plays no role in module-id discovery.

>(That means you could even cache the module ID
>in a per-objfile data structure to pay the price only once.)

The aix_tls_objfile_data struct attached via aix_tls_objfile_data_key implements exactly this. After the first call for a given shared library, cache.resolved is true and subsequent calls return cache.mod_id immediately without re-scanning.

Please see
+struct aix_tls_objfile_data
+{
+  bool resolved = false;
+  uint64_t mod_id = 0;
+};

One more thing I want to tell is testing in my LPAR with the test case patch, I found that the AIX loader assigns module-id 0 to initial-exec shared libraries, not just the main executable. The old code treated mod_id == 0 as "not yet allocated" and threw an error, which broke access to any variable in a library loaded as initial-exec. The v3 version of this patch removes those checks, and adds rs6000_aix_find_initial_exec_tls_offset to handle the fact that for initial-exec libraries the static XCOFF symbol value differs from the runtime TP-relative offset by the size of the initial TLS layout shift.

So these are the changes I made and am sending the v3 version of the patch to address the same.

So we have three patches:
v3-0001-Add-TLS-variable-debug-support-for-AIX-64-bit-XCO.patch - Already sent
v3-0002-This-patch-adds-support-to-debug-thread-local-var.patch - With corrected things to review
v2-0003-Add-test-cases-for-all-TLS-support-in-AIX.patch -  with corrected commit message.

Kindly let me know what you think once I send them. Thanks for the guidance and review again.

Have a nice day ahead.

Thanks and regards,
Aditya.

[-- Attachment #2: Type: text/html, Size: 9184 bytes --]