Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add define for MIPS_XHASH #471

Closed
wants to merge 1 commit into from
Closed

Add define for MIPS_XHASH #471

wants to merge 1 commit into from

Conversation

LinuxUserGD
Copy link

Fixes undeclared identifier error on musl libc (Gentoo amd64/clang), see https://mail-index.netbsd.org/source-changes-hg/2021/05/01/msg290415.html

clang++ -std=c++17 -DPACKAGE_NAME=\"patchelf\" -DPACKAGE_TARNAME=\"patchelf\" -DPACKAGE_VERSION=\"0.17.2\" -DPACKAGE_STRING=\"patchelf\ 0.17.2\" -DPACKAGE_BUGREPORT=\"\" -DPACKAGE_URL=\"\" -DPACKAGE=\"patchelf\" -DVERSION=\"0.17.2\" -DHAVE_CXX17=1 -I.    -Wall -std=c++17 -D_FILE_OFFSET_BITS=64     -O3 -pipe -march=skylake -mtune=skylake -D_FORTIFY_SOURCE=3 -stdlib=libc++ -c -o patchelf.o patchelf.cc
patchelf.cc:1087:33: error: use of undeclared identifier 'DT_MIPS_XHASH'
            } else if (d_tag == DT_MIPS_XHASH) {
                                ^
1 error generated.
make[1]: *** [Makefile:380: patchelf.o] Error 1
make[1]: Leaving directory '/var/tmp/portage/dev-util/patchelf-0.17.2/work/patchelf-0.17.2/src'
make: *** [Makefile:448: all-recursive] Error 1

Fixes error: use of undeclared identifier 'DT_MIPS_XHASH' on musl libc
@tgbugs
Copy link

tgbugs commented Mar 7, 2023

Is it safe to define that constant if it has not already been defined? Is it a glibc specific constant or is it an implementation of something that is standard on MIPS?

@LinuxUserGD
Copy link
Author

LinuxUserGD commented Mar 7, 2023

Is it safe to define that constant if it has not already been defined? Is it a glibc specific constant or is it an implementation of something that is standard on MIPS?

It's a constant in binutils so probably not specific to glibc-only systems I think

Edit: Also seems to be defined in

#define DT_MIPS_XHASH 0x70000036

@brenoguim
Copy link
Collaborator

Edit: Also seems to be defined in

#define DT_MIPS_XHASH 0x70000036

Since this is already defined, why would this PR be necessary? Perhaps the real problem is another one?

@LinuxUserGD
Copy link
Author

Edit: Also seems to be defined in

#define DT_MIPS_XHASH 0x70000036

Since this is already defined, why would this PR be necessary? Perhaps the real problem is another one?

Maybe elf.h from musl libc is included first https://git.musl-libc.org/cgit/musl/tree/include/elf.h which doesn't define DT_MIPS_XHASH so _ELF_H is already defined without constants from https://github.com/NixOS/patchelf/blob/master/src/elf.h

There's also a gentoo bug https://bugs.gentoo.org/860888 removing DT_MIPS_XHASH which might be a better approach.

@brenoguim
Copy link
Collaborator

I'm confused. So even though patchelf contains the elf.h file used to build and test, it needs to also work with some other elf.h? It seems backwards to me.
If the problem is indeed musl providing another elf.h (needs verification) it would be nicer if it just didn't unless we explicitly ask for it.
Let me try to reproduce the issue and better understand what is going on.

@LinuxUserGD
Copy link
Author

LinuxUserGD commented Mar 11, 2023

I'm confused. So even though patchelf contains the elf.h file used to build and test, it needs to also work with some other elf.h? It seems backwards to me. If the problem is indeed musl providing another elf.h (needs verification) it would be nicer if it just didn't unless we explicitly ask for it. Let me try to reproduce the issue and better understand what is going on.

Ok, Gentoo actually removes removes the file which causes the build failure: https://gitweb.gentoo.org/repo/gentoo.git/tree/dev-util/patchelf/patchelf-0.17.2.ebuild#n17
Sry, seems like code should be fine upstream, I guess a downstream gentoo patch might be better then.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants