{"thread":{"id":"60879","subject":"libification: how to avoid symbol collisions?","startedAt":"2024-02-09T02:30:37Z","lastAt":"2024-02-13T16:50:24Z","messageCount":6,"participants":["Kyle Lippincott","rsbecker@nexbridge.com","Junio C Hamano"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"488263","messageId":"CAO_smVji5aGjx1V-EGbumRRpOuGY0SkXZUn9g4LxKmMO3aw=Sg@mail.gmail.com","threadId":"60879","inReplyTo":null,"subject":"libification: how to avoid symbol collisions?","fromName":"Kyle Lippincott","fromEmail":"spectral@google.com","sentAt":"2024-02-09T02:30:22Z","receivedAt":"2024-02-09T02:30:37Z","isPatch":false,"sender":{"key":"spectral@google.com","avatar":"https://avatars.githubusercontent.com/u/6371650?v=4"},"body":"While thinking about libification, I was wondering what we can/need to\ndo about symbols (specifically functions, since our libraries will\nlikely have few to no extern variables) that need to escape their\ntranslation unit (.c file) but that we don't want to risk colliding\nwith symbols from our \"host\" project.\n\nFor any header that we're offering up as an API boundary we can have\nprefixed names, but there are symbols from git-compat-util.h with\nsimple and likely common names like `die`, `usage`, error`, etc. I'm\nfar from an expert on linkers, but I'm under the impression that even\nthough we'll only be #including git-compat-util.h in our own .c files\n(so the compiler for the host project wouldn't see them), the produced\nstatic library will still be \"providing\" these symbols unless we mark\nthem as `static` (and if we mark them as `static`, they can't be used\noutside of their translation unit). This means that if the host\nproject has a version of `die` (or links against yet another library\nthat does), we run into what C++ calls a One Definition Rule (ODR)\nviolation: we have two providers of the symbol `die` with different\nimplementations, and the behavior is undefined (no error needs to be\ngenerated as far as I know).\n\nWith dynamic libraries I believe that we have more control over what\ngets exposed, but I don't know of functionality for this when linking\nstatically. I'm assuming there is no such functionality, as projects\nlike openssl (ty Randall for mentioning this) appear to have a\nconvention of prefixing the symbols they put in their \"public\" API\n(i.e. in non-internal header files) with things like OSSL_, and of\nprefixing the symbols they put in their \"private\" APIs that can't be\nmarked as `static` with `ossl_`. I'd love to be wrong about this. :)\n\nIf I'm right that this is an issue, does this imply that we'd need to\nrename every non-static function in the git codebase that's part of a\nlibrary to have a `git_` prefix, even if it won't be used outside of\nthe git project's own .c files? Is there a solution that doesn't\ninvolve making it so that we have to type `git_` a billion times a day\nthat's also maintainable? We could try to guess at how likely a name\ncollision would be and only do this for ones where it's obviously\ngoing to collide, but if we get it wrong, I'm concerned that we'll run\ninto subtle ODR violations that *at best* erode confidence in our\nlibrary, and can actually cause outages, data corruption, and\nsecurity/privacy issues.\n"},{"id":"488288","messageId":"000301da5b5c$5477d7f0$fd6787d0$@nexbridge.com","threadId":"60879","inReplyTo":"CAO_smVji5aGjx1V-EGbumRRpOuGY0SkXZUn9g4LxKmMO3aw=Sg@mail.gmail.com","subject":"RE: libification: how to avoid symbol collisions?","fromName":"","fromEmail":"rsbecker@nexbridge.com","sentAt":"2024-02-09T13:31:41Z","receivedAt":"2024-02-09T13:31:55Z","isPatch":false,"sender":{"key":"randall.becker@nexbridge.ca","avatar":"https://avatars.githubusercontent.com/u/28956764?v=4"},"body":"On Thursday, February 8, 2024 9:30 PM, Kyle Lippincott wrote:\n>While thinking about libification, I was wondering what we can/need to do about\n>symbols (specifically functions, since our libraries will likely have few to no extern\n>variables) that need to escape their translation unit (.c file) but that we don't want\n>to risk colliding with symbols from our \"host\" project.\n>\n>For any header that we're offering up as an API boundary we can have prefixed\n>names, but there are symbols from git-compat-util.h with simple and likely\n>common names like `die`, `usage`, error`, etc. I'm far from an expert on linkers, but\n>I'm under the impression that even though we'll only be #including git-compat-\n>util.h in our own .c files (so the compiler for the host project wouldn't see them),\n>the produced static library will still be \"providing\" these symbols unless we mark\n>them as `static` (and if we mark them as `static`, they can't be used outside of their\n>translation unit). This means that if the host project has a version of `die` (or links\n>against yet another library that does), we run into what C++ calls a One Definition\n>Rule (ODR)\n>violation: we have two providers of the symbol `die` with different\n>implementations, and the behavior is undefined (no error needs to be generated as\n>far as I know).\n>\n>With dynamic libraries I believe that we have more control over what gets exposed,\n>but I don't know of functionality for this when linking statically. I'm assuming there\n>is no such functionality, as projects like openssl (ty Randall for mentioning this)\n>appear to have a convention of prefixing the symbols they put in their \"public\" API\n>(i.e. in non-internal header files) with things like OSSL_, and of prefixing the symbols\n>they put in their \"private\" APIs that can't be marked as `static` with `ossl_`. I'd love\n>to be wrong about this. :)\n>\n>If I'm right that this is an issue, does this imply that we'd need to rename every non-\n>static function in the git codebase that's part of a library to have a `git_` prefix, even\n>if it won't be used outside of the git project's own .c files? Is there a solution that\n>doesn't involve making it so that we have to type `git_` a billion times a day that's\n>also maintainable? We could try to guess at how likely a name collision would be\n>and only do this for ones where it's obviously going to collide, but if we get it wrong,\n>I'm concerned that we'll run into subtle ODR violations that *at best* erode\n>confidence in our library, and can actually cause outages, data corruption, and\n>security/privacy issues.\n\nI think we only need to do this for functions that are in the libification code-base for non-static symbols (and any data elements that may end up in a DLL some day). The bulk of the non-libified code base would only need to adapt to new symbol names if those symbols are specifically moved. die(), error(), are probably going to be impacted, but they can be aliased with #defines internally to git to git_die() or git_error(), for example.\n--Randall\n\n"},{"id":"488301","messageId":"xmqqil2ximxq.fsf@gitster.g","threadId":"60879","inReplyTo":"CAO_smVji5aGjx1V-EGbumRRpOuGY0SkXZUn9g4LxKmMO3aw=Sg@mail.gmail.com","subject":"Re: libification: how to avoid symbol collisions?","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2024-02-09T17:09:53Z","receivedAt":"2024-02-09T17:09:59Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Kyle Lippincott <spectral@google.com> writes:\n\n> If I'm right that this is an issue, does this imply that we'd need to\n> rename every non-static function in the git codebase that's part of a\n> library to have a `git_` prefix, even if it won't be used outside of\n> the git project's own .c files? Is there a solution that doesn't\n> involve making it so that we have to type `git_` a billion times a day\n> that's also maintainable? We could try to guess at how likely a name\n> collision would be and only do this for ones where it's obviously\n> going to collide, but if we get it wrong, I'm concerned that we'll run\n> into subtle ODR violations that *at best* erode confidence in our\n> library, and can actually cause outages, data corruption, and\n> security/privacy issues.\n\nIf you end up with a helper function name \"foo\" that is defined in\nour X.c and used by our Y.c but is not part of the published \"git\nlibrary API\", we may want to rename it so that such a common name\ncan be used by programs that link with the \"git library\".  We may\nchoose to rename it to \"GitLib_foo\".\n\nDo we want to keep the source of our library code, which defines the\nhelper function \"foo\" in X.c and calls it in Y.c, intact so that the\nhelper is still named \"foo\" as far as we are concerned?  Or do we\n\"bite the bullet\" and bulk modify both the callers and the callee?\n\nI'd imagine that we would rather avoid such a churn at all cost [*].\nAfter all, \"foo\" is *not* supposed to be callable by any human\nwritten code, and that is why we rename it to a name \"GitLib_foo\"\nthat is unlikely to overlap with any sane human would use.\n\n\tSide note: if a public API function that we want our library\n\tclients to call is misnamed, we want to rename it so that we\n\twould both internally and externally use the same public\n\tname, I would imagine.\n\nThe mechanics to rename should be a solved problem, I think, as we\nare obviously not the first project that wants to build a library.\n\nIf the names are all simple, we could do this in CPP, i.e. invent a\nheader file that has bunch of such renames like\n\n    #define foo GitLib_foo\n\nand include it in both X.c and Y.c.  But \"foo\" may also appear as\nthe name of a type, a member in a structure, etc., where we may not\nwant to touch, so in a project larger than toy scale, this approach\nmay not work well.\n\n\"objcopy --redefine-sym\" would probably be a better way.  I haven't\nwritten a linker script, but I heard rumors that there is RENAME()\nto rename symbols, and using that might be another avenue.\n\n\n"},{"id":"488499","messageId":"CAO_smVg-xxVeevuPr_E2F9mjnDzN7Tq6o9Tkhy4c+9qBmQTxdw@mail.gmail.com","threadId":"60879","inReplyTo":"000301da5b5c$5477d7f0$fd6787d0$@nexbridge.com","subject":"Re: libification: how to avoid symbol collisions?","fromName":"Kyle Lippincott","fromEmail":"spectral@google.com","sentAt":"2024-02-13T01:02:37Z","receivedAt":"2024-02-13T01:02:55Z","isPatch":false,"sender":{"key":"spectral@google.com","avatar":"https://avatars.githubusercontent.com/u/6371650?v=4"},"body":"On Fri, Feb 9, 2024 at 5:31 AM <rsbecker@nexbridge.com> wrote:\n>\n> On Thursday, February 8, 2024 9:30 PM, Kyle Lippincott wrote:\n> >While thinking about libification, I was wondering what we can/need to do about\n> >symbols (specifically functions, since our libraries will likely have few to no extern\n> >variables) that need to escape their translation unit (.c file) but that we don't want\n> >to risk colliding with symbols from our \"host\" project.\n> >\n> >For any header that we're offering up as an API boundary we can have prefixed\n> >names, but there are symbols from git-compat-util.h with simple and likely\n> >common names like `die`, `usage`, error`, etc. I'm far from an expert on linkers, but\n> >I'm under the impression that even though we'll only be #including git-compat-\n> >util.h in our own .c files (so the compiler for the host project wouldn't see them),\n> >the produced static library will still be \"providing\" these symbols unless we mark\n> >them as `static` (and if we mark them as `static`, they can't be used outside of their\n> >translation unit). This means that if the host project has a version of `die` (or links\n> >against yet another library that does), we run into what C++ calls a One Definition\n> >Rule (ODR)\n> >violation: we have two providers of the symbol `die` with different\n> >implementations, and the behavior is undefined (no error needs to be generated as\n> >far as I know).\n> >\n> >With dynamic libraries I believe that we have more control over what gets exposed,\n> >but I don't know of functionality for this when linking statically. I'm assuming there\n> >is no such functionality, as projects like openssl (ty Randall for mentioning this)\n> >appear to have a convention of prefixing the symbols they put in their \"public\" API\n> >(i.e. in non-internal header files) with things like OSSL_, and of prefixing the symbols\n> >they put in their \"private\" APIs that can't be marked as `static` with `ossl_`. I'd love\n> >to be wrong about this. :)\n> >\n> >If I'm right that this is an issue, does this imply that we'd need to rename every non-\n> >static function in the git codebase that's part of a library to have a `git_` prefix, even\n> >if it won't be used outside of the git project's own .c files? Is there a solution that\n> >doesn't involve making it so that we have to type `git_` a billion times a day that's\n> >also maintainable? We could try to guess at how likely a name collision would be\n> >and only do this for ones where it's obviously going to collide, but if we get it wrong,\n> >I'm concerned that we'll run into subtle ODR violations that *at best* erode\n> >confidence in our library, and can actually cause outages, data corruption, and\n> >security/privacy issues.\n>\n> I think we only need to do this for functions that are in the libification code-base for non-static symbols (and any data elements that may end up in a DLL some day).\n\nI believe the hope is that the majority/entirety of plumbing code will\nbe provided as a library, and we'll likely want to have a significant\nportion of porcelain code as well. I think we're really talking about\n(effectively) \"all of git\", but not all at once. If we attempt to make\nthings safe based on guesses about what's likely to collide with other\nproject's code, we'll (a) get it wrong, and only discover later when\nthey try to add our library to their project, and (b) have a\nmaintenance burden, where we now have to think about every function\nname we introduce, which would not be fun (and we'll get it wrong.\nFrequently.)\n\n> The bulk of the non-libified code base would only need to adapt to new symbol names if those symbols are specifically moved.\n\nI'm not following what you mean by \"moved\" here.\n\n> die(), error(), are probably going to be impacted, but they can be aliased with #defines internally to git to git_die() or git_error(), for example.\n> --Randall\n>\n"},{"id":"488501","messageId":"CAO_smVizKLL2NHFBpszJn+ieJhCEZyvvOT-BWv6Oz5pGiafPVg@mail.gmail.com","threadId":"60879","inReplyTo":"xmqqil2ximxq.fsf@gitster.g","subject":"Re: libification: how to avoid symbol collisions?","fromName":"Kyle Lippincott","fromEmail":"spectral@google.com","sentAt":"2024-02-13T02:48:26Z","receivedAt":"2024-02-13T02:48:44Z","isPatch":false,"sender":{"key":"spectral@google.com","avatar":"https://avatars.githubusercontent.com/u/6371650?v=4"},"body":"On Fri, Feb 9, 2024 at 9:10 AM Junio C Hamano <gitster@pobox.com> wrote:\n>\n> Kyle Lippincott <spectral@google.com> writes:\n>\n> > If I'm right that this is an issue, does this imply that we'd need to\n> > rename every non-static function in the git codebase that's part of a\n> > library to have a `git_` prefix, even if it won't be used outside of\n> > the git project's own .c files? Is there a solution that doesn't\n> > involve making it so that we have to type `git_` a billion times a day\n> > that's also maintainable? We could try to guess at how likely a name\n> > collision would be and only do this for ones where it's obviously\n> > going to collide, but if we get it wrong, I'm concerned that we'll run\n> > into subtle ODR violations that *at best* erode confidence in our\n> > library, and can actually cause outages, data corruption, and\n> > security/privacy issues.\n>\n> If you end up with a helper function name \"foo\" that is defined in\n> our X.c and used by our Y.c but is not part of the published \"git\n> library API\", we may want to rename it so that such a common name\n> can be used by programs that link with the \"git library\".  We may\n> choose to rename it to \"GitLib_foo\".\n\nIf it's internal, we may want to name it with a different prefix than\nGitLib, if we expect the exposed API of the library to have this\nprefix, just as a signal to readers where the internal/external\nboundaries are.\n\n>\n> Do we want to keep the source of our library code, which defines the\n> helper function \"foo\" in X.c and calls it in Y.c, intact so that the\n> helper is still named \"foo\" as far as we are concerned?  Or do we\n> \"bite the bullet\" and bulk modify both the callers and the callee?\n>\n> I'd imagine that we would rather avoid such a churn at all cost [*].\n> After all, \"foo\" is *not* supposed to be callable by any human\n> written code, and that is why we rename it to a name \"GitLib_foo\"\n> that is unlikely to overlap with any sane human would use.\n>\n>         Side note: if a public API function that we want our library\n>         clients to call is misnamed, we want to rename it so that we\n>         would both internally and externally use the same public\n>         name, I would imagine.\n>\n> The mechanics to rename should be a solved problem, I think, as we\n> are obviously not the first project that wants to build a library.\n>\n> If the names are all simple, we could do this in CPP,\n\nAt first I thought you meant C++, and I was like \"Yeah, that's a\npossible solution: when building a library, compile it as C++ with\nname mangling, except for the symbols we intend to export!\" -- this\nwas not what you meant, though. Kind of amusingly, that idea might\nwork, and might even be maintainable once we got to that state, but\ngetting to that state would be a lot of cleanup because of C++'s\nstricter type system (`char *p = ptr;`, where `ptr` is a `void*` for\nexample; maybe this is a call to malloc or similar). Since the git\nlibraries don't exist yet, there's technically no worries about\nbackwards compatibility with requiring a C++ compiler.\n\n> i.e. invent a\n> header file that has bunch of such renames like\n>\n>     #define foo GitLib_foo\n>\n> and include it in both X.c and Y.c.  But \"foo\" may also appear as\n> the name of a type, a member in a structure, etc., where we may not\n> want to touch, so in a project larger than toy scale, this approach\n> may not work well.\n\nGlancing at the tags file, it looks like there's a small number of\ncases where this would be problematic, and they're mostly things where\nthere's a function named the same thing as either a struct variable\nstoring the result of the function. So it could work, but there's over\n3,500 symbols (if I did my filtering of the tags file correctly) that\nare not scoped to a specific file (i.e. static), or\nstruct/enum/typedef/union names. That's going to be quite annoying to\nmaintain; even if we don't end up having to do all 3,500 symbols, for\nthe files that are part of some public library, we'd add maintenance\nburden because we'd need to remember to either make every new function\nbe static, or add it to this list. I assume we could create a test\nthat would enforce this (\"static, named with <prefix>, or added to\n<list>\"), so the issue is catchable, but it will be exceedingly\nannoying every time one encounters this.\n\n>\n> \"objcopy --redefine-sym\" would probably be a better way.  I haven't\n> written a linker script, but I heard rumors that there is RENAME()\n> to rename symbols, and using that might be another avenue.\n>\n>\n\nI'd thought of linker scripts, but rejected the idea due to\nassumptions I made about their portability - this could be mitigated\nby having a linker-script-generator step in the build process, but\nthis seems difficult to maintain. It also implies the same maintenance\nburden as the #defines, where when introducing a new function to X.c\nthat is called from Y.c we'd have to edit the list of \"symbols to\nrename\".\n"},{"id":"488538","messageId":"xmqqr0hg8g1k.fsf@gitster.g","threadId":"60879","inReplyTo":"CAO_smVizKLL2NHFBpszJn+ieJhCEZyvvOT-BWv6Oz5pGiafPVg@mail.gmail.com","subject":"Re: libification: how to avoid symbol collisions?","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2024-02-13T16:50:15Z","receivedAt":"2024-02-13T16:50:24Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Kyle Lippincott <spectral@google.com> writes:\n\n> struct/enum/typedef/union names. That's going to be quite annoying to\n> maintain; even if we don't end up having to do all 3,500 symbols, for\n> the files that are part of some public library, we'd add maintenance\n> burden because we'd need to remember to either make every new function\n> be static, or add it to this list. I assume we could create a test\n> that would enforce this (\"static, named with <prefix>, or added to\n> <list>\"), so the issue is catchable, but it will be exceedingly\n> annoying every time one encounters this.\n\nNo matter how we do this, we'd need to maintain that list, so the\nchoices are between \"#define\" and \"objcopy --redefine-sym\" if we\nlimit ourselves to static linking, I think.  The former may be more\nportable but makes me feel dirty.  The debuggers will not see the\nnames we want to use, for one thing.  \"rename selected symbols in *.o\nfiles\" approach, if it can be done on platforms we want the lib thing\non, would be more preferable.\n\nWe also should be able to create a single linkable object (e.g., Z.o\nout of X.o and Y.o from the previous example---it could be Z.so that\nis dynamically linked at runtime) to resolve the symbols that need\nto be shared only among the object files (like \"foo\" that is defined\nin X.o whose address is needed in Y.o) and after such a linking is\ndone, these \"internal\" symbols can be stripped away from the\nresulting object file.  For that, we'd also need to maintain that\nlist of internal symbols that are needed in order to link our\nobjects together.\n"}]}