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

Misc minor fixes #182

Merged
merged 15 commits into from
Nov 11, 2021
Merged

Misc minor fixes #182

merged 15 commits into from
Nov 11, 2021

Conversation

vathpela
Copy link
Contributor

@vathpela vathpela commented Nov 9, 2021

Here are a bunch of relatively minor fixes.

This adds:
.gdb_history - gdb command history
.C - sometimes the output of gcc -E
/.cache/ - clangd cache
.cer - x509 certs
.esl - efi signature lists
compile_commands.json - bear / clangd / etc helper

Signed-off-by: Peter Jones <pjones@redhat.com>
Copy link
Member

@frozencemetery frozencemetery left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • "make: Temporarily disable gcc -flto": not sure I see how making efivar/efisecdb separate is related to LTO. Just removing the -flto from OPTIMIZE_GCC seems to avoid the issue for me.
  • "make: remove makeguids dependency from 'make clean'": this seems to have other changes folded in, including some abixml? Possibly I'm misunderstanding the origin of the problem.

All else seems okay to me.

Currently, with binutils 2.37, linking with -flto is broken:
> gcc -Og -flto -g3 -Wall -Wextra  -Werror  -std=gnu11 -funsigned-char -fvisibility=hidden -specs=/home/pjones/devel/github.com/efivar/security/src/include/gcc.specs -fno-merge-constants  -L.   -Wl,--add-needed -Wl,--build-id -Wl,--no-allow-shlib-undefined -Wl,--no-undefined-version -Wl,-z,now -Wl,-z,muldefs -Wl,-z,relro -Wl,--fatal-warnings     -DLIBEFIVAR_VERSION=37 -D_GNU_SOURCE -I/home/pjones/devel/github.com/efivar/security/src/include/  -shared -Wl,-soname,libefivar.so.1 -Wl,--version-script=libefivar.map  \
>   -o libefivar.so crc32.o dp.o dp-acpi.o dp-hw.o dp-media.o dp-message.o efivarfs.o error.o export.o guid.o guids.o guid-symbols.o lib.o vars.o -ldl
> collect2: fatal error: ld terminated with signal 11 [Segmentation fault], core dumped
> compilation terminated.
> make[1]: *** [/home/pjones/devel/github.com/efivar/security/src/include/rules.mk:32: libefivar.so] Error 1

This commit disables -flto temporarily until the fix[0] is more widely
deployed, and forces the compiler to make the .o files for efivar and
efisecdb independently.  It also switches to specifying .o instead of .c
on the command line, because gcc won't allow multiple .c files without
-flto, and makes the "% : %.o" rule use $(sort $^) instead of just $^ in
order to de-duplicate, so we're not trying to link makeguids.o into
makeguids twice.

[0] https://sourceware.org/bugzilla/show_bug.cgi?id=28264

Signed-off-by: Peter Jones <pjones@redhat.com>
@vathpela
Copy link
Contributor Author

vathpela commented Nov 10, 2021

* "make: Temporarily disable gcc -flto": not sure I see how making efivar/efisecdb separate is related to LTO.  Just removing the `-flto` from OPTIMIZE_GCC seems to avoid the issue for me.

EFIVAR_OBJECTS was just for consistency, but without doing the MAKEGUIDS_OBJECTS change, I was getting gcc command lines like: gcc ... -o makeguids makeguids.c makeguids.c guids.c, which then has two problems: without -flto multiple sources can't be compiled that way, and also one file is listed twice. Switching to objects fixes one, and the $(sort $^) rule de-duplicates. I'll add this to the commit message so it's less confusing.

* "make: remove makeguids dependency from 'make clean'": this seems to have other changes folded in, including some abixml?  Possibly I'm misunderstanding the origin of the problem.

Part of the problem is the way the stuff in guid.c gets used in makeguids, with the horrible EFIVAR_BUILD_ENVIRONMENT hack to make it work. Moving those to being static inlines that are then wrapped by our ABI stubs gets us out of needing that hack there (we still need it to avoid including the missing header file from makeguids).

But you're right in that I accidentally merged the memcmp->efi_guid_cmp fix in at the same time; I'll split that out.

@vathpela vathpela force-pushed the misc-minor-fixes branch 2 times, most recently from f237c61 to f4eb757 Compare November 10, 2021 22:19
This just updates the files to a newer version of abidw.

Signed-off-by: Peter Jones <pjones@redhat.com>
For a long time, we've compared guids and their symbolic names with
memcmp().  That provides a stable sort order, and very little
(if anything) depends on the sorted order of a list of GUIDs for
anything other than searching.

Unfortunately, when you're actually looking at the sorted output of a
list of guids, this winds up being incredibly confusing, because the
first element of a guid is a little endian uint23_t, which means
memcmp() is evaluating the bytes in a different order than sorting a
text file full of guids would.

This patch switches to comparing the internal integral members of the
guid as unsigned integers, which is still slightly confusing when it
comes to the 4th one (it is compared opposite of how it appears, because
GUIDs are terrible), and to comparing the symbolic names with strncmp().

Signed-off-by: Peter Jones <pjones@redhat.com>
This will keep makeguids from being built when you run 'make clean',
which was annoying.

Signed-off-by: Peter Jones <pjones@redhat.com>
Signed-off-by: Peter Jones <pjones@redhat.com>
Signed-off-by: Peter Jones <pjones@redhat.com>
Signed-off-by: Peter Jones <pjones@redhat.com>
Signed-off-by: Peter Jones <pjones@redhat.com>
Signed-off-by: Peter Jones <pjones@redhat.com>
Signed-off-by: Peter Jones <pjones@redhat.com>
Signed-off-by: Peter Jones <pjones@redhat.com>
Signed-off-by: Peter Jones <pjones@redhat.com>
Some of the systems we build in now make us explicitly list gcc and
make, so here they are.

Signed-off-by: Peter Jones <pjones@redhat.com>
This adds a couple of guids to our libefivar ABI, and also adds a
checker so builds will fail if more are incorrectly added.

Signed-off-by: Peter Jones <pjones@redhat.com>
[rharwood@redhat.com: commit message tweak]
@frozencemetery frozencemetery merged commit b4b14d4 into rhboot:main Nov 11, 2021
@vathpela vathpela deleted the misc-minor-fixes branch November 11, 2021 21:21
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.

None yet

2 participants