search

kaoh / globalplatform

C library + command-line for Open- / GlobalPlatform smart cards
https://kaoh.github.io/globalplatform/
Other
73 stars 29 forks source link

Build failure when building with LTO #95

Closed jas4711 closed 2 months ago

jas4711 commented 2 months ago

Hi! I noticed this bug report:

https://bugs.launchpad.net/ubuntu/+source/globalplatform/+bug/2060262

Indeed I can reproduce the build error by enabling LTO with gcc.

They proposed this patch:

https://launchpadlibrarian.net/723353780/globalplatform.debdiff

Since this modify the upstream source code, I wanted to forward this bug report upstream so you can consider it. I'm not certain this is the right fix for this problem, review from someone with more cmake and LTO knowledge would be nice.

I will use the patch in the Debian packages so they build with LTO going forward.

Thanks, /Simon

koh-osug commented 2 months ago

I have tested this and the test does not finish with these settings on Ubuntu 22.04 x86_64:

Running tests... Test project /home/widerstand/projects/globalplatform Start 1: scp03Test

So I would assume that also the Debian test packages cannot execute the tests anymore. What is the need for using LTO? Could the tests not work without this optimization if the other parts are still using it?

koh-osug commented 2 months ago

I have the same behavior randomly also without the LTO flags that the test get stuck. So not related to this. Maybe the mock library I'm using has issues with libc.

koh-osug commented 2 months ago

OK, the lto patch seems to be fine. Independent of it the tests get stuck sometimes.

jas4711 commented 2 months ago

Yes I think that is unrelated, because it builds fine on many platforms with the patch:

https://buildd.debian.org/status/package.php?p=globalplatform https://salsa.debian.org/pkg-security-team/globalplatform/-/jobs/5926374

However I don't understand enough about LTO vs cmocka vs this library to tell if this is a good patch or not. However since it only affects the test suite, it seems unlikely to do a lot of harm to the main library/tool, so that's why I opted to add it.

koh-osug commented 2 months ago

I pushed it to the master.

jas4711 commented 2 months ago

Thank you! We will find out if it causes any trouble later on...