Bug 247121

Summary: StringOperators.cpp test re-defines WTF::StringTypeAdapter implementation, undefined behaviour
Product: WebKit Reporter: Kimmo Kinnunen <kkinnunen>
Component: Web Template FrameworkAssignee: Kimmo Kinnunen <kkinnunen>
Status: RESOLVED FIXED    
Severity: Normal CC: webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
See Also: https://bugs.webkit.org/show_bug.cgi?id=177566
https://bugs.webkit.org/show_bug.cgi?id=63330
Bug Depends on:    
Bug Blocks: 246947    

Description Kimmo Kinnunen 2022-10-27 05:34:20 PDT
StringOperations.cpp test re-defines WTF::StringTypeAdapter implementation, undefined behaviour

StringOperations.cpp re-defines the template implementations in order to count the String copies.
This is undefined behaviour. Two different compilation units linked to same binary cannot have two different implementations for the same name symbol.

The linker knows of two different implementations by the same name, and may do whatever.

Consider:

// header.h
inline int myFunc()
{
  WTF_STRINGTYPEADAPTER_COPIED_WTF_STRING(); 
  return 1;
}

// impl1.cpp:
  #define WTF_STRINGTYPEADAPTER_COPIED_WTF_STRING() (void())
  #include "header.h"
  void call1() {
      myFunc();
  }

// impl2.cpp:
  #define WTF_STRINGTYPEADAPTER_COPIED_WTF_STRING() printf("kimmo")
  #include "header.h"
  void call2() {
    myFunc();
  }


// main.cpp
  void call1();
  void call2();
  int main() {
    call1();
    call2();
    return 0;
  }

c++ impl1.cpp impl2.cpp main.cpp -o my-undefined

It's unclear what the app does.

Currently, all invocations of TestWTF seem to use the test-overridden hack of the implementation.
Comment 1 Radar WebKit Bug Importer 2022-10-28 01:18:05 PDT
<rdar://problem/101669240>
Comment 2 Kimmo Kinnunen 2022-10-28 02:12:46 PDT
Pull request: https://github.com/WebKit/WebKit/pull/5893
Comment 3 EWS 2022-10-29 00:49:24 PDT
Committed 256131@main (d4f69df7d5b1): <https://commits.webkit.org/256131@main>

Reviewed commits have been landed. Closing PR #5893 and removing active labels.