[gklib] Fix build error with Android triplet

[Cheney-W]

Fixes #36791

No feature need to be tested.

• Changes comply with the maintainer guide.
• SHA512s are updated for each updated download.
• The "supports" clause reflects platforms that may be fixed by this new version.
• Any fixed CI baseline entries are removed from that file.
• Any patches that are no longer applied are deleted from the port's directory.
• The version database is fixed by rerunning ./vcpkg x-add-version --all and committing the result.
• Only one version is added to each modified port's versions file.

[dg0yt]

Hm, this adds a declaration, but where is the definition if not on Linux/glibc?

[Cheney-W]

Thank you for pointing that out. My original assumption was that these two functions wouldn't be called when not on Linux/glibc, so I didn't consider whether they were actually defined.
Now I've added conditions to them, so they're only declared if the execinfo.h header file can be found.
This is the original text in the source's CMake: check_include_file(execinfo.h HAVE_EXECINFO_H)

[dg0yt]

If you know you have execinfo.h, and these functions are declared in execinfo.h, why don't you #include <execinfo.h>?

[Cheney-W]

I've considered the situation you mentioned. In the issue I submitted to the upstream, I mentioned two possible solutions, one is to add #include <execinfo.h>, but I noticed that the upstream source already has the following inclusion relationship:
error.c have #include <GKlib.h>
GKlib.h have #include "gk_arch.h"
gk_arch.h have:

#ifdef HAVE_EXECINFO_H
#include <execinfo.h>
#endif

So, instead of directly including the header file, I opted for using extended function declarations.

[Cheney-W]

Of course, these are just my personal thoughts. If you think it's better to directly include the header file, I'll adjust my PR accordingly.

[dg0yt]

Wait, the fix is for android, not for clang-17?!
Then the tentative fix is there:
https://github.com/microsoft/vcpkg/pull/35851/files#diff-95c803e4c056c6f158a02350ea41a27b7c646b9571048daff3707d20be342f68

[dg0yt]

i.e. it needs API level 33.

[BillyONeal]

I've considered the situation you mentioned. In the issue I submitted to the upstream, I mentioned two possible solutions, one is to add #include <execinfo.h>, but I noticed that the upstream source already has the following inclusion relationship: error.c have #include <GKlib.h> GKlib.h have #include "gk_arch.h" gk_arch.h have:

#ifdef HAVE_EXECINFO_H
#include <execinfo.h>
#endif

So, instead of directly including the header file, I opted for using extended function declarations.

That suggests that HAVE_EXECINFO_H was off and therefore this patch doesn't do anything?

[Cheney-W]

The error-prone code is also confined within #ifdef HAVE_EXECINFO_H,

#ifdef HAVE_EXECINFO_H
...
  size = backtrace(array, 10);
  strings = backtrace_symbols(array, size);
...
#endif

so when HAVE_EXECINFO_H is off, the error will not be triggered, and there is no need for patching work.

[dg0yt]

The PR's title is misleading. It is not a clang 17 issue.

The truth is that on Android execinfo.h can be present (detected) but still nthe functions are not available until raising the API level.

And the fix proposed here fixes the static lib build, but users will hit the wall when they try to link executables.

[Cheney-W]

@BillyONeal Is this modification the final solution to this issue? If it is, I will close my PR.

[BillyONeal]

@BillyONeal Is this modification the final solution to this issue? If it is, I will close my PR.

I'm not sure this is correct either; it depends on how that macro gets actually used.

[dg0yt]

I'm not sure this is correct either; it depends on how that macro gets actually used.

I wouldn't have introduced the API level check if I hadn't tested it that time.
It somewhat mirrors my original comment here: It is not enough to see a declaration --- or header. You need an implementation.

[Cheney-W]

Duplicate of #37877