1 comment:
Patch Set #1, Line 22: strlen
I wasn't aware that libc marks its symbols weak (and I'm not sure that this is the case everywhere).
Actually, I mixed up things a little bit here. _weak_ attribute won't affect the order in which linker looks for symbols resolution. And as you said - this is not the case everywhere for libc.
The real thing which happens, is that linker (all linkers I have tried so far, however I don't think that this is obligatory/forced in any spec) looks for symbols firstly in the files provided explicitly as an input. Only if there are still some unresolved ones, linker will search in other libraries (like libc). Thus, when providing src/lib/string.c on command line for gcc, we are (should be) safe.
One option to go at this in a structured way could be to rename symbols in string.o during linking or with objcopy, but that gets tedious real quick (you have to carry around a full list of that kind of symbols).
Yes, this is the "most proper" option I believe. But something hard to implement and maintain.
> For testing that it picks up the right symbols (and that libc really takes the backseat), we could do as follows:
>
> 1. Add some non-sense implementation of a libc function (e.g. a strlen that always returns 42) to tests/,
> 2. Add a test that uses this function and expects it to return 42 no matter the (non-42 char length) input.
>
> That way we have a signal indicating that the tests are looking at the right thing in the normal case, too, right? If we see that there are major libc implementations that fail this, we can still reconsider our options.
Every test is a separate binary (especially input files are listed explicitly for each test) and tests are isolated from each other. Even if we will create one test, which is really checking that we are linking against correct implementation, this doesn't necessarily mean, that other binaries are built the way we really want it to.
We would need to link such a fake function against every test touching coreboot's libc implementation. Of course, we cannot use strlen() or any other function defined in coreboot inside unit under test. As a consequence - we wouldn't really check which one is invoked. In other words - vicious circle...
That's all not for this commit, but please put it on your backlog.
Sounds reasonable. Let me think about some other ideas, how we may tackle this problem.
To view, visit change 40538. To unsubscribe, or for help writing mail filters, visit settings.