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

Fix potential ODR violation due to weak symbols being overridden #231

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

bobsayshilol
Copy link

Reproducer:

// g++ repro.cpp edlib.cpp -o repro

#include "edlib.h"
#include <cstdio>

struct Block {
  Block();
};

// Shouldn't be called because no instance of `Block` is created in this file.
Block::Block() { printf("This shouldn't be called\n"); }

int main() {
  EdlibAlignConfig config{};
  config.mode = EDLIB_MODE_SHW;
  EdlibAlignResult result = edlibAlign("a", 1, "b", 1, config);
  edlibFreeAlignResult(result);
}
# Note that the same result is observed when using clang++ too.
$ g++ repro.cpp edlib.cpp -o repro
$ ./repro
This shouldn't be called

The issue is that Block's constructor is exported as a weak symbol from whichever translation unit has the contents of edlib.cpp since it defaults to external linkage. To fix this we wrap the types in an anonymous namespace to give them internal linkage and hide them from any client code.

`static` is already applied to the functions which hides them from any
user code, but you can't put `static` on types so they have to go in an
anonymous namespace instead.

This fixes weak symbols (ie constructors) leaking out and being
overridden by client code
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.

1 participant