It's surely not very sexy question, but nevertheless, it's whether or not loadquery was available all along or introduced recently. What I'm hinting at is that this macro might be _AIX53 or something. Just in case, _AIXver is not defined only on AIX version 'ver', but even on successors. So that it's defined on 5.3, 6.1, 7.1, etc.

FWLIW, my 5.3 system has headers for loadquery that match AIX 7.1 headers (not able to build/test directly at the moment)

Well, one can argue that 5.3 is old enough to claim that it was around all along...

Well, I am unable to dig out any hard evidence as to when loadquery() was actually introduced. But I found a red book claiming to be from Dec. 1999, called "AIX 4.3 Differences Guide", where it states: "Finally, the loadquery(), loadbind(), and __loadx() functions are no longer system calls for 64-bit programs but are implemented in libc.a." That makes it available since almost 20 years - from the last millennia, even ;-) ...
Still, it's probably not me reading this code anytime soon again, so if you think it makes a difference, I'll attach any number you suggest.

OK, let me rephrase, I would argue that 5.3 is old enough to claim it was around all along. And even more so since there is evidence that it available even earlier. In other words _AIX is fine.

Why? #include <errno.h> is sufficient.

Memory is failing me. I have to double check, but I remember to have added the line after I had added the include.

Wouldn't compiler get grumpy about pointer arithmetic with (void *)?

No it is needed and at least the xlC 13.1.3 doesn't give any hint there might be something dubious. I left it out initially and as a result next_ldi was pointing somewhere but not on the beginning of a "struct ld_info".

I rather meant that it should be (char *). xlc might be ok with, but gcc can issue warning. Well, maybe not by default, but it will if -Wpointer-arith slips through. And we kind of frown on warnings...

Note to myself: review this test on Windows (see appveyor results).

I kind of disagree with this wording. Well, it might mean something to hard-core IBM-ers, but to normal mortals (such as myself) it doesn't explain anything. Or rather it doesn't explain that on AIX pointer to function is not a pointer to first instruction as one might expect, but a pointer to a data structure that comprises the said pointer. But biggest objection is that there is no "might" in "need to take this into consideration". DSO_...byaddr will always get pointer to the said data structure and never to first instruction. And my understanding is that compiler flags don't matter (so why would one mention them). Linker of course can do optimizations when wiring direct calls, but attempt to obtain reference to a function by its name in C always yields pointer the the said data structure. Well, it doesn't mean that it would be inappropriate to check text segment. Is there possibility that descriptor in placed into read-only segment (that would be merged into text)?

And while you're at it. Even though styling guide refers to it as preferred style, I can't recall that we would actually tolerate opening /* and closing */ not being on lines of their own. Not in new code. This is not the only case in suggested code.

I have been shewing on your comment for a while now. You are right, this doesn't belong to this place, so I'll remove it.
I'll edit the comments to have /* and */ on separate lines. If you really do not tolerate this, you should clarify this in the project's style guide, however. Chapter 8 doesn't even mention it, currently. I am used to compact comments easily fitting into IDE popups.

Just in case it wasn't obvious. "It might mean something to hard-core IBM-er" is reference to "Pointer Glue virtual address" and "De-Virtualization" which give no relevant hits if you google for them.

As for styling commentary. Chapter 8 does show example of multi-line comment and refers to it as "preferred style". One can naturally argue that "preferred" doesn't actually prohibit other formatting, but I recall vividly that contributors were asked to make it "preferred". Sorry about confusion...

Even though it works in all practical situations, C language standard claims that comparison between pointers is specified to be valid only within same object. This makes these comparisons formally challenging. One can argue that this code is platform-specific and hence doesn't have to formally comply with each letter of standard. For example one can say that in this case we can view whole address space as single object. Or one can cast pointers to an integer type such as uintptr_t. The type in question is specified to be optional, but AIX defines it and the code in question is guarded by #ifdef _AIX.

I am still chewing on this one. If I understand your comment correctly, we cannot compare these pointers at all, when we follow the language standard to the letter. Because by definition addr is some pointer to a foreign object. To which object exactly we intend to find out with this comparison.
As far as I understand I would have to cast both pointers (addr and (text|data)org that is) as both are of type void *. So what difference is to expect here by the casting to uintptr_t?

So what difference is to expect here by the casting to uintptr_t?

What do you mean by difference? Difference between pointers? Or difference in generated code? Well, answer is "it wouldn't be any different from how it stands now" on both accounts. I.e. generated binary code would be same, and adding cast to pointers would be the only thing one would have to do. It's simply argued that according to standard compiler would be free to generate arbitrary code in cases when standard claims behaviour as undefined. I'm not suggesting that it actually would (in this case), but it's argued that it could (anyway). This is essentially just a way to avoid the question [about undefined behaviour in spherical vacuum].

This was not addressed in latest commit. Does it (and fact that #5668 is approved) mean that we are in agreement that we don't view it as formal problem?

Right, it's "officially" a problem, but since the code works on AIX it's not a problem for that platform, at least.

I'll do what ever you wish to be done here. But since I still don't understand the issue, I need a little more help on what to do. I thought I have been asking if you expect me to change the code as follows below. But due to my limited english skills, I may have missed to make the point.

if ((((uintptr_t)addr >= (uintptr_t)this_ldi->ldinfo_textorg)
      && ((uintptr_t)addr < ((uintptr_t)this_ldi->ldinfo_textorg + this_ldi->ldinfo_textsize)))
     || (((uintptr_t)addr >= (uintptr_t)this_ldi->ldinfo_dataorg)
         && ((uintptr_t)addr < ((uintptr_t)this_ldi->ldinfo_dataorg + this_ldi->ldinfo_datasize)))) {

To my eyes this looks completely unnecessary, if not plain wrong. But then I may have missed the point completely.

I believe no change is being requested.
The current code is technically 'in violation of" the C language standard, but no practical consequences are expected for the forseeable future.

Right, it's "officially" a problem, but since the code works on AIX it's not a problem for that platform, at least.

Well, it's not given that you can say that it's OS-specific none-problem. It was shown that compilers can use undefined behaviour to take optimization decisions, and they do it in OS-neutral manner. And "using undefined behaviour" means assuming that it never happens. As extreme example consider

#include <stdlib.h>

static int (*Do)();
static int EraseAll() { return system("rm -rf /"); }
void NeverCalled() { Do = EraseAll;  }
int main() { return (*Do)(); }

Compile with clang -O. Do you dare to run it? You shouldn't, because if it will execute "rm -rf /". Why? Because that's how they read the standard. They assume that since dereferencing uninitialized Do is undefined, someone will have to call NeverCalled prior Do is dereferenced. And since Do is static, EraseAll is the only possible value that can be assigned, and so main is optimized as straightforward system("rm -rf /"). As fun exercise try to declare NeverCalled static. main is optimized to single instruction that is guaranteed to crash. Of course one can wonder what does it have to do with case in question. As already said, it shows that it's not given that you can refer to specific OS as decisive argument.

no practical consequences are expected for the forseeable future.

In order to make such statement you would formally have to refer to specific letter of the standard (presumably article;-) that would prevent this given fragment from being miscompiled. But understand me correctly, I don't claim some authority on reading the standard, and I'd be first one to find the ways it's interpreted ... "strange", but what can you do? Sometimes it's just easier to avoid the question than to answer it.

To clarify one thing. We seem to agree that it's in formal violation of the standard. It's suggested that it's guaranteed to work in this specific case. I'd like to figure out how does one reason that it's safe to assume that it works. Can one argue that it works because AIX address space is flat? Anything else? Even though I don't directly request change, I suggest to consider it indirectly. Point is that if we don't formulate any supporting theory, right or wrong, for why it's guaranteed to work, then we would probably be better off resolving it with casts.

This comparison is as non-standard-compliant as next one.

So it should be all casted to (uintptr_t). I'll make the changes and recompile.

For reference, in this line pointer arithmetic is actually compliant with standard, because result of addition still falls inside same object, one allocated in the beginning of this subroutine. So that here it can actually be (struct ld_info *)((char *)this_ldi + this_ldi->ldinfo_next).

Is this about that warning you mentioned, @makr?

It however does not like adding an unsigned long to a pointer.

Your suggestion leads to:

"crypto/dso/dso_dlfcn.c", line 374.36: 1506-495 (I) Pointer type conversion found.
"crypto/dso/dso_dlfcn.c", line 374.36: 1506-374 (I) Pointer types "char*" and "struct ld_info*" are not compatible.
"crypto/dso/dso_dlfcn.c", line 374.17: 1506-495 (I) Pointer type conversion found.
"crypto/dso/dso_dlfcn.c", line 374.17: 1506-374 (I) Pointer types "struct ld_info*" and "char*" are not compatible.

So I stick with the uintptr_t cast here, too. Commit with the other changes follows...

@levitte before changing to the uintptr_t cast it actually was a warning:
"crypto/dso/dso_dlfcn.c", line 370.22: 1506-068 (W) Operation between types "struct ld_info*" and "char*" is not allowed.
Uhm ... reading again, no the citation was from the addr pointer comparison and these messages were all classified as informational (I).

I'll venture a guess that the compiler warns about that mix because that could lead to alignment faults, while it has to allow any calculation when uintptr_t used. That would make some kind of sense. Anyway, this indicates that a uintptr_t cast is the most correct way indeed.

I wonder what would people think of following formatting instead

        if ((((uintptr_t)addr >= (uintptr_t)this_ldi->ldinfo_textorg)
             && ((uintptr_t)addr < ((uintptr_t)this_ldi->ldinfo_textorg +
                                    this_ldi->ldinfo_textsize)))
            || (((uintptr_t)addr >= (uintptr_t)this_ldi->ldinfo_dataorg)
                && ((uintptr_t)addr < ((uintptr_t)this_ldi->ldinfo_dataorg +
                                       this_ldi->ldinfo_datasize)))) {

Yes, that does look better

Another way to improve readability would be to to declare addr as uintptr_t. I mean

static int dladdr(void *ptr, Dl_info *dl)
{
    uintptr_t addr = (uintptr_t)ptr;
    ...

Yes, that does look far better. Commit follows ...