3 ms·

Yeah, but the program may not be broken until you, the glibc maintainer, calls `raise(SIGSEGV)`. Most programs using setenv call it before starting any threads

Yeah, but the program may not be broken until you, the glibc maintainer, calls `raise(SIGSEGV)`. Most programs using setenv call it before starting any threads. That is not broken. Detecting the linkage of thread support and crashing that program on purpose is, frankly, a pathological way to fix a non-broken program. Besides which, your proposal won't work anyway, because this remains a potential problem in single threaded programs anyway: a program calling getenv, storing the result, and then calling setenv on the same variable and using the previously stored result will break anyway. In summary, your proposal is broken in two different ways: 1) it breaks well-defined programs, and 2) it fails to break broken programs.

jeroenhd 3y ago
I wouldn't implement it during linkage, obviously single threaded putenv/setenv calls should still be permitted as part of initialisation routines. Count the number if children in /proc/self/task for all I care, the detection needs to happen during runtime. You're right that putenv/setenv are also horribly broken in other ways, and doing multi thread detection doesn't prevent those problems. In a perfect world we would just kill off these two functions all together, replacing them with either crashes or no-ops, but that'd be an even harder sell.
- lelanthran 3y ago
  > I wouldn't implement it during linkage, obviously single threaded putenv/setenv calls should still be permitted as part of initialisation routines. Count the number if children in /proc/self/task for all I care, the detection needs to happen during runtime. That still breaks well-defined, non-broken programs which don't call getenv/setenv in racing ways. There is no way for you do a conditional-upon-threads mechanism without false positives. > You're right that putenv/setenv are also horribly broken in other ways, and doing multi thread detection doesn't prevent those problems. In a perfect world we would just kill off these two functions all together, replacing them with either crashes or no-ops, but that'd be an even harder sell. But you don't need to in order to meet your original goal - breaking programs which do call setenv/getenv in the wrong order. Proposing to remove them altogether doesn't fulfill the goal of finding the breakages immediately and introduces breakages in existing programs. My alternative: use LD_PRELOAD and provide alternative setenv/getenv functions which raise SIGSEGV when setenv is called on a variable more than once, and when getenv is called on a variable that was already setted once. It requires nothing more than a counter for each of setenv/getenv per variable. That finds programs which actually are broken, with no false positives, and ignores threads altogether because they don't matter under the counter system[1]. Best of all, you can implement this in an afternoon, without needing to modify glibc, and then test it with every single executable on your system to see which ones break.[2] [1] Since the caller knows they are not thread-safe anyway, we aren't looking for the error where the caller calls setenv concurrently in different threads. That's a different problem. [2] I would wager good money that few, if any systems, will break under this test.

jeroenhd 3y ago

I wouldn't implement it during linkage, obviously single threaded putenv/setenv calls should still be permitted as part of initialisation routines. Count the number if children in /proc/self/task for all I care, the detection needs to happen during runtime. You're right that putenv/setenv are also horribly broken in other ways, and doing multi thread detection doesn't prevent those problems. In a perfect world we would just kill off these two functions all together, replacing them with either crashes or no-ops, but that'd be an even harder sell.

lelanthran 3y ago
> I wouldn't implement it during linkage, obviously single threaded putenv/setenv calls should still be permitted as part of initialisation routines. Count the number if children in /proc/self/task for all I care, the detection needs to happen during runtime. That still breaks well-defined, non-broken programs which don't call getenv/setenv in racing ways. There is no way for you do a conditional-upon-threads mechanism without false positives. > You're right that putenv/setenv are also horribly broken in other ways, and doing multi thread detection doesn't prevent those problems. In a perfect world we would just kill off these two functions all together, replacing them with either crashes or no-ops, but that'd be an even harder sell. But you don't need to in order to meet your original goal - breaking programs which do call setenv/getenv in the wrong order. Proposing to remove them altogether doesn't fulfill the goal of finding the breakages immediately and introduces breakages in existing programs. My alternative: use LD_PRELOAD and provide alternative setenv/getenv functions which raise SIGSEGV when setenv is called on a variable more than once, and when getenv is called on a variable that was already setted once. It requires nothing more than a counter for each of setenv/getenv per variable. That finds programs which actually are broken, with no false positives, and ignores threads altogether because they don't matter under the counter system[1]. Best of all, you can implement this in an afternoon, without needing to modify glibc, and then test it with every single executable on your system to see which ones break.[2] [1] Since the caller knows they are not thread-safe anyway, we aren't looking for the error where the caller calls setenv concurrently in different threads. That's a different problem. [2] I would wager good money that few, if any systems, will break under this test.