Hacker News Board Hacker News, browsed like a classic message board
Live data from Hacker News

Fedora 38 LLVM vs. Team Fortress 2

airlied.blogspot.com

Post reply on HN
Print view View original

71–80 of 111 posts

Re: Fedora 38 LLVM vs. Team Fortress 2

#71

Post by 0x457 »

bravetraveler wrote: post #39
gabcoh wrote: post #35

Earlier quoted context omitted.

Can Linux not trivially do the same thing as windows with LD_PRELOAD? If so why is this more of an issue on Linux than Windows? Is it really less a technical challenge and more just a matter of Linux getting less support from upstream developers?

I was thinking/wondering this myself. Not to reinvent the wheel - more toss an idea around, but a 'venv for LD_PRELOAD' sounds like it'd deal with this pretty handily Not... in a way I'd use as a distribution/release maintainer. Probably as an administrator [of my LAN]

That's Nix with extra steps.

Re: Fedora 38 LLVM vs. Team Fortress 2

#72

Post by DannyBee »

AnssiH wrote: post #52
DannyBee wrote: post #48

Fedora 38 includes the LLVM15 libs to maintain backwards compatibility. Why is this automatically using a new, incompatible solib, instead of a versioned solib?

The LLVM dependency is in the HW-specific driver solib which is loaded by the OpenGL library, which has the same soname as before.

Okay, then why does it fail when it should then still use llvm15?

The author states this is an llvm16 issue, but unless the driver was built against llvm16, it should still be loading llvm15.

If it was built against llvm16 (or loads llvm16), and doesn't work, that's not a failure of anything other than QA testing.

Re: Fedora 38 LLVM vs. Team Fortress 2

#73

Post by mjg59 »

admax88qqq wrote: post #32

Unfortunately this is exactly the type of stuff that makes supporting commercial apps on linux a nightmare. Weird crashes due to weird linking of system libraries. Common distros are very adamant about dynamic linking everything in order to support the use case of "core library has vulnerability, upgrade it in place without rebuilding consuming apps." Along with a desire to avoid "dll hell" and force a single canonic…

This is largely solved with approaches such as Flatpak or Snap, but graphics drivers are still an issue - they're expected to be supplied by the distribution, and components of them end up in-process in the application even if the rest of the application's runtime is shipped with the application. If there's an incompatibility between the application runtime and assumptions made by that driver code (as there appears to be in this case - TF2 ships its own malloc() implementation, but the graphics driver code ends up using it inconsistently and so blows up) then you're going to have problems.

I don't think there's anything about Windows that would fundamentally change things here. Windows apps aren't shipping their own graphics drivers, even if they're bundling everything else.

Re: Fedora 38 LLVM vs. Team Fortress 2

#74

Post by MichaelZuo »

eikenberry wrote: post #60
admax88qqq wrote: post #32

Unfortunately this is exactly the type of stuff that makes supporting commercial apps on linux a nightmare. Weird crashes due to weird linking of system libraries. Common distros are very adamant about dynamic linking everything in order to support the use case of "core library has vulnerability, upgrade it in place without rebuilding consuming apps." Along with a desire to avoid "dll hell" and force a single canonic…

Isn't this exactly the use case for which flatpaks are designed? Isn't Redhat/Fedora in the process of adopting them as the primary way to support third party/proprietary graphical apps like Steam? Doesn't the current Steam flatpak avoid this issue? TLDR; isn't this already addressed?

It would appear so, I don't understand why this blog post is so popular.

Re: Fedora 38 LLVM vs. Team Fortress 2

#75

Post by xioxox »

josephg wrote: post #68
nly wrote: post #53

Earlier quoted context omitted.

You can compile binaries with additional relative library paths in to them that will take priority over /usr/lib64

How? Maybe this should be better documented & recommended. I suppose at some point you're just statically linking with more steps - though for a problem like this it might be worth it.

See the ELF rpath, which can be set by the linker. This can be modified after using patchelf.

Re: Fedora 38 LLVM vs. Team Fortress 2

#76

Post by boutique »

Funnily enough, on Half-Life 1 engine-based games (i.e. the engine that came before HL2 - on which Team Fortress 2 runs; such as Counter-Strike 1.6), a different allocator problem exists -- glibc's malloc() just decides to fail miserably[0] on some setups.

[0] https://github.com/ValveSoftware/halflife/issues/3158

Re: Fedora 38 LLVM vs. Team Fortress 2

#77

Post by kimixa »

boutique wrote: post #76

Funnily enough, on Half-Life 1 engine-based games (i.e. the engine that came before HL2 - on which Team Fortress 2 runs; such as Counter-Strike 1.6), a different allocator problem exists -- glibc's malloc() just decides to fail miserably[0] on some setups. [0] https://github.com/ValveSoftware/halflife/issues/3158

that's exactly the sort of error you get if something has written just out of bounds on a malloc'd chunk - it clobbers the allocator's internal state, which appears to be what that assert() is checking.

It's probably an allocation before the failing one that is being misued - so the backtrace pointing to openal doesn't necessarily mean it's openal's fault.

Running with valgrind or another heap memory checking tool will probably be helpful to track down that particular linked bug.

EDIT:

It looks like that there's at least one out-of-bounds write when starting up half life (On arch linux, so maybe slightly different library versions and not loaded the counterstrike mod).

It looks like a valve bug - writing 2 bytes at index [30] of a malloc'd size of 31 goes one byte over, and it looks from the backtrace it's all valve's code and not deep in some library that might have been loaded in. Writing 2 bytes to a string is a bit odd, perhaps it's trying to null-terminate but somehow uses a wstring null? Or some attempt at SIMD that isn't correctly bound?

It doesn't seem to crash for me though, it might just be luck that nothing important is put 1 byte over, and it feels a bit unlikely something would be due to allocation and type alignment requirements, but it's perfectly valid for the malloc implementation to keep something important in that byte.

Or perhaps there's some other dynamics that change this - it looks like it's doing stuff with paths, so may change size (of the allocation or even the amount written) based on where the steam app is installed - stuff like your user name length changing that may be the difference between a crash. Or even another issue somewhere else I didn't see, or valgrind didn't catch.

Just goes to show how many games ship for years with "big" bugs :P

For reference:

  ==27467== Invalid write of size 2                                                                                                                                                                                                            
  ==27467==    at 0x406526A: GetSteamContentPath() 
  (pathmatch.cpp:523)
  ==27467==    by 0x4065927: pathmatch(char const*, char\*, 
  bool, char*, unsigned int) [clone .part.1] (pathmatch.cpp:594)
  ==27467==    by 0x4066849: pathmatch (pathmatch.cpp:541)
  ==27467==    by 0x4066849: CWrap (pathmatch.cpp:685)
  ==27467==    by 0x4066849: __wrap___xstat (pathmatch.cpp:907)
  ==27467==    by 0x406294A: stat (stat.h:455)
  ==27467==    by 0x406294A: CFileSystem_Stdio::FS_stat(char const*, stat*) (FileSystem_Stdio.cpp:225)
  ==27467==    by 0x4060819: CBaseFileSystem::AddPackFiles(char const*) (BaseFileSystem.cpp:1325)
  ==27467==    by 0x4060AA4: CBaseFileSystem::AddSearchPathInternal(char const*, char const*, bool) (BaseFileSystem.cpp:254)
  ==27467==    by 0x4060B37: CBaseFileSystem::AddSearchPath(char const*, char const*) (BaseFileSystem.cpp:186)
  ==27467==    by 0x8049003: main (launcher.cpp:413)
  ==27467==  Address 0x45e5f4e is 30 bytes inside a block of size 31 alloc'd
  ==27467==    at 0x4041714: malloc (vg_replace_malloc.c:393)
  ==27467==    by 0x4357C4A: strdup (strdup.c:42)
  ==27467==    by 0x42F1A76: realpath_stk (canonicalize.c:410)
  ==27467==    by 0x42F1A76: realpath@@GLIBC_2.3 (canonicalize.c:432)
  ==27467==    by 0x406525B: GetSteamContentPath() (pathmatch.cpp:520)
  ==27467==    by 0x4065927: pathmatch(char const*, char\*, bool, char*, unsigned int) [clone .part.1] (pathmatch.cpp:594)
  ==27467==    by 0x4066849: pathmatch (pathmatch.cpp:541)
  ==27467==    by 0x4066849: CWrap (pathmatch.cpp:685)
  ==27467==    by 0x4066849: __wrap___xstat (pathmatch.cpp:907)
  ==27467==    by 0x406294A: stat (stat.h:455)
  ==27467==    by 0x406294A: CFileSystem_Stdio::FS_stat(char const*, stat*) (FileSystem_Stdio.cpp:225)
  ==27467==    by 0x4060819: CBaseFileSystem::AddPackFiles(char const*) (BaseFileSystem.cpp:1325)
  ==27467==    by 0x4060AA4: CBaseFileSystem::AddSearchPathInternal(char const*, char const*, bool) (BaseFileSystem.cpp:254)
  ==27467==    by 0x4060B37: CBaseFileSystem::AddSearchPath(char const*, char const*) (BaseFileSystem.cpp:186)
  ==27467==    by 0x8049003: main (launcher.cpp:413)

Re: Fedora 38 LLVM vs. Team Fortress 2

#78

Post by cesarb »

mjg59 wrote: post #73
admax88qqq wrote: post #32

Unfortunately this is exactly the type of stuff that makes supporting commercial apps on linux a nightmare. Weird crashes due to weird linking of system libraries. Common distros are very adamant about dynamic linking everything in order to support the use case of "core library has vulnerability, upgrade it in place without rebuilding consuming apps." Along with a desire to avoid "dll hell" and force a single canonic…

This is largely solved with approaches such as Flatpak or Snap, but graphics drivers are still an issue - they're expected to be supplied by the distribution, and components of them end up in-process in the application even if the rest of the application's runtime is shipped with the application. If there's an incompatibility between the application runtime and assumptions made by that driver code (as there appears t…

> TF2 ships its own malloc() implementation, but the graphics driver code ends up using it inconsistently and so blows up) then you're going to have problems. I don't think there's anything about Windows that would fundamentally change things here.

Yes, there is: on Windows, due to the way DLL linking works there, the graphics driver wouldn't use the malloc() implementation from TF2. The flat linking namespace in which you can globally replace the memory allocator for every dynamic library does not exist on Windows; if the graphics driver is linked to the memory allocator from the C library, it will get the memory allocator from that C library, not from some other DLL in the same process.

That's not to say Windows is free of dynamic linking problems. While on Linux it's mostly only NSS and the graphics driver (and only when explicitly requested), on Windows it's common for unrelated third party software to inject DLLs and threads all over every process on the system. And it's not uncommon for these injected DLLs to do things like hooking into system DLLs (by overwriting the entry point of exported functions, or even internal functions), leading to hard-to-diagnose crashes when things are not like they expected.

Re: Fedora 38 LLVM vs. Team Fortress 2

#79

Post by eklitzke »

cesarb wrote: post #78
mjg59 wrote: post #73

Earlier quoted context omitted.

This is largely solved with approaches such as Flatpak or Snap, but graphics drivers are still an issue - they're expected to be supplied by the distribution, and components of them end up in-process in the application even if the rest of the application's runtime is shipped with the application. If there's an incompatibility between the application runtime and assumptions made by that driver code (as there appears t…

> TF2 ships its own malloc() implementation, but the graphics driver code ends up using it inconsistently and so blows up) then you're going to have problems. I don't think there's anything about Windows that would fundamentally change things here. Yes, there is: on Windows, due to the way DLL linking works there, the graphics driver wouldn't use the malloc() implementation from TF2. The flat linking namespace in whi…

Linux does not have a flat linking namespace (for example, see RTLD_NEXT in the dlsym man page). If it did, this wouldn't be a problem because everything would use the libc malloc or everything would use the tcmalloc implementation. It's just that glibc exports pretty much everything as a weak symbol, most programs and libraries that link against libc bind to whatever ld.so picks as the first in the search object search order, and messing with linker settings is the last thing most developers want to think about.

I just read TFA so I haven't dived into all the details but my wild speculation is that the graphics driver is doing an aligned new and an unaligned delete. The alignment parameters in the new and delete give them different symbol names but you're free to implement them by dispatching to the unaligned version, so this wouldn't necessarily cause a problem (even in C++17 mode) if the malloc implementation doesn't actually handle them differently. However if tcmalloc was built for C++14 then it would only have the unaligned operators, and therefore the aligned new would resolve to the glibc implementation and the unaligned delete would resolve to the tcmalloc implementation, or something like that.

Re: Fedora 38 LLVM vs. Team Fortress 2

#80

Post by olliej »

viraptor wrote: post #67
olliej wrote: post #61

Earlier quoted context omitted.

Haha, you've missed the issue. The question is what does the system do when someone overrides the builtin allocator functions, but does not override all of them. You are absolutely correct that as a developer you can have your process override the allocator functions, and that is in fact what TF has done. The problem is that they have not overridden all of the allocation functions, and so they're crashing due to mism…

I was going for "ignore the issue, let's just re-patch all alloc/free pointers, built-in or external, new or old" which I think would still work. (As long as anticheat doesn't freak out) It wouldn't suffer from inconsistencies, because you'd control all the calls again. Or is there something missing in this approach?

You can't repatch all the calls. The OS/standard library provides a set of global operator new and delete implementations, and for largely historical reasons they are _required_ to allow processes to override them with their own implementations.

Now when a program does decide that they're going to override the global operator new and delete functions the standard library is required to default to them instead. So generally the standard library exposes them as weak symbols, and the OS and stdlib links to them by symbol name. That way on program launch the program's version of the operator new/delete symbols are what win. So that's how the OS and standard library are able to interact with the program despite it overriding what is ostensibly the system allocator.

So in principle the OS could simply make sure that the user provided operator new, delete, etc are always directed to the system allocator routines. The problem is that when compiling user code there's no obligation to call the user provided new, delete, etc through a symbol, and in general won't. Instead the calls will generally be compiled down to PC relative loads and branches as those are significantly faster. The net result is that while the OS _could_ force the symbols to always resolve to the system functions, things would break due to the user code still using the user specified functions, but those functions then would not be compatible and the result would be sadness. Hence the user defined operator implementations have to win.

The problem is what happens when not all of the operators are overloaded. This historically hasn't been a problem: there's the plain and [] variants, which can be overloaded independently, and the no_throw variants of each which have in practice not been an issue because the way those are implemented by default is essentially

    try { return ::operator new(size); } catch(...) { return nullptr; }
So does just directly the operator new that people override.

The problem that operator new(size_t, align_val_t) is that depending on your compiler flags you will get different versions of ::operator new being called, and because of the alignment requirements the aligned operator new can't just forward to the default new implementation. So introducing it is the first time failure of a program to implement the full suite of operators results in an actual runtime error vs minor inefficiency.

The reality is that the minimally correct solution is for all programs and libraries that overload the global allocation operators to override all of them. The better solution is for these programs and libraries to stop overloading the global allocators.

Many years ago (talking >a decade at this point) when webkit first adopted a non-default allocator it overloaded the global operators. Perhaps unsurprisingly this caused issues, and now webkit (and presumably blink) do the correct thing: there's a standard base class (FastAllocated or something) that defines operator new, delete, and the [] variants, and using that as a base class results in the non-default allocator.

Post reply on HN