Move from deprecated codecvt to ICU for Unicode handling on non-Windows platforms (#210)

* Move from codecvt to ICU on non-Windows platforms.
* Update setup.py to point to libicu instead of codecvt
* Update documentation
* Fix CI jobs to pull in libicu as needed on MacOS and Linux
* Use clang-format from llvm repo

Co-authored-by: Emilio López

Author: gsutherland-trailofbits

.github/workflows/ci.yml

@@ -4,6 +4,7 @@ on:
   push:
     branches:
       - master
+      - libicu-test
   pull_request:
   schedule:
     # run CI every day even if no PRs/merges occur
@@ -17,8 +18,10 @@ jobs:
 
       - name: deps
         run: |
+          wget -qO- https://apt.llvm.org/llvm-snapshot.gpg.key | sudo tee /etc/apt/trusted.gpg.d/apt.llvm.org.asc
+          sudo add-apt-repository -y "deb http://apt.llvm.org/noble/ llvm-toolchain-noble-18 main"
           sudo apt-get update
-          sudo apt-get install -y clang-format
+          sudo apt-get install -y clang-format-18 libicu-dev
 
       - name: lint
         run: |
@@ -44,6 +47,19 @@ jobs:
     - uses: actions/checkout@v4
       with:
         submodules: 'true'
+    - name: Install ICU (Ubuntu)
+      if: matrix.platform == 'ubuntu-latest'
+      run: |
+        sudo apt-get update
+        sudo apt-get install -y libicu-dev
+    - name: Install ICU (macOS)
+      if: matrix.platform == 'macos-latest'
+      run: |
+        brew install icu4c
+    - name: Set ICU_ROOT for macOS
+      if: matrix.platform == 'macos-latest'
+      run: |
+        echo "ICU_ROOT=$(brew --prefix icu4c)" >> $GITHUB_ENV
     - name: Enable ASan+UBSan Sanitizers
       if: matrix.build-type == 'Debug'
       run: |
@@ -85,6 +101,16 @@ jobs:
     - uses: actions/setup-python@v5
       with:
         python-version: ${{ matrix.python }}
+    - name: Install ICU (Ubuntu)
+      if: matrix.platform == 'ubuntu-latest'
+      run: |
+        sudo apt-get update
+        sudo apt-get install -y libicu-dev
+    - name: Install ICU (macOS)
+      if: matrix.platform == 'macos-latest'
+      run: |
+        brew install icu4c
+        echo "ICU_ROOT=$(brew --prefix icu4c)" >> $GITHUB_ENV
     - name: build
       run: |
         python -m pip install build wheel setuptools

README.md

@@ -52,6 +52,16 @@ More information about `pepy` can be found in its [README](./pepy/README.md).
 
 ## Dependencies
 
+### ICU
+
+ICU is required on non-Windows platforms (Linux, macOS). **You do not need ICU on Windows.**
+
+- ICU library (International Components for Unicode)
+  - Debian/Ubuntu: `sudo apt-get install libicu-dev`
+  - RedHat/Fedora: `sudo dnf install libicu-devel`
+  - macOS: `brew install icu4c`
+  - vcpkg: `vcpkg install icu`
+
 ### CMake
   * Debian/Ubuntu: `sudo apt-get install cmake`
   * RedHat/Fedora: `sudo yum install cmake`
@@ -91,6 +101,17 @@ cmake -G "Visual Studio 16 2019" -A Win64 ..
 cmake --build . --config Release
 ```
 
+### MacOS-specific
+
+When ICU is installed via brew it will not be in the usual library path, so you will need to set the `ICU_ROOT` environment variable to the location where ICU is installed when running cmake.
+
+The path is usually `/opt/homebrew/opt/icu4c@version`, where `version` is the version number of ICU you have installed.
+
+```
+ICU_ROOT=/opt/homebrew/opt/icu4c@123 cmake -DCMAKE_BUILD_TYPE=Release ..
+ICU_ROOT=/opt/homebrew/opt/icu4c@123 cmake --build .
+```
+
 ## Testing
 
 You can build the (catch2-based) tests by adding `-DPEPARSE_ENABLE_TESTING=ON` during CMake configuration. Build, and then run with `ctest` or `cmake --build . --target test`.

pe-parser-library/CMakeLists.txt

@@ -14,14 +14,13 @@ list(APPEND PEPARSERLIB_SOURCEFILES
 )
 
 # NOTE(ww): On Windows we use the Win32 API's built-in UTF16 conversion
-# routines; on other platforms we use codecvt. codecvt is nominally deprecated
-# in C++17 and onwards, but will probably be available for quite some time.
-# Previous versions of pe-parse used ICU when available, but this caused
-# DLL hell on Windows and wasn't worth the additional dependency.
+# routines; on other platforms we use ICU (International Components for Unicode).
+# Previous versions used codecvt, which was deprecated in C++17.
 if(MSVC)
   list(APPEND PEPARSERLIB_SOURCEFILES src/unicode_winapi.cpp)
 else()
-  list(APPEND PEPARSERLIB_SOURCEFILES src/unicode_codecvt.cpp)
+  find_package(ICU COMPONENTS uc REQUIRED)
+  list(APPEND PEPARSERLIB_SOURCEFILES src/unicode_libicu.cpp)
 endif()
 
 add_library(${PROJECT_NAME} ${PEPARSERLIB_SOURCEFILES})
@@ -39,6 +38,11 @@ target_include_directories(
 )
 target_compile_options(${PROJECT_NAME} PRIVATE ${GLOBAL_CXXFLAGS})
 
+# Link ICU on non-Windows platforms
+if(NOT MSVC)
+  target_link_libraries(${PROJECT_NAME} PRIVATE ICU::uc)
+endif()
+
 install(
   TARGETS ${PROJECT_NAME}
   EXPORT pe-parse-config

pe-parser-library/src/unicode_codecvt.cpp pe-parser-library/src/unicode_libicu.cpppe-parser-library/src/unicode_codecvt.cpp renamed to pe-parser-library/src/unicode_libicu.cpp

@@ -22,15 +22,37 @@ OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
 THE SOFTWARE.
 */
 
-#include <codecvt>
-#include <locale>
 #include <pe-parse/to_string.h>
+#include <unicode/ustring.h>
+#include <unicode/utypes.h>
 
 namespace peparse {
-// See
-// https://stackoverflow.com/questions/38688417/utf-conversion-functions-in-c11
 std::string from_utf16(const UCharString &u) {
-  std::wstring_convert<std::codecvt_utf8<char16_t>, char16_t> convert;
-  return convert.to_bytes(u);
+  if (u.empty()) {
+    return std::string();
+  }
+
+  const UChar *src = reinterpret_cast<const UChar *>(u.data());
+  int32_t srcLength = static_cast<int32_t>(u.size());
+
+  // First pass: determine required buffer size
+  UErrorCode status = U_ZERO_ERROR;
+  int32_t destLength = 0;
+  u_strToUTF8(nullptr, 0, &destLength, src, srcLength, &status);
+
+  if (status != U_BUFFER_OVERFLOW_ERROR && U_FAILURE(status)) {
+    return std::string(); // Return empty on error (matches current behavior)
+  }
+
+  // Second pass: perform actual conversion
+  std::string result(static_cast<std::size_t>(destLength), '\0');
+  status = U_ZERO_ERROR;
+  u_strToUTF8(&result[0], destLength, nullptr, src, srcLength, &status);
+
+  if (U_FAILURE(status)) {
+    return std::string();
+  }
+
+  return result;
 }
 } // namespace peparse

pepy/pepy.cpp

@@ -42,18 +42,20 @@ using namespace peparse;
     return ((pepy_##OBJ *) self)->ATTR;                                     \
   }
 
-#define OBJECTGETTER(OBJ, ATTR, DOC)                         \
-  {                                                          \
-    (char *) #ATTR, (getter) pepy_##OBJ##_get_##ATTR,        \
-        (setter) pepy_attr_not_writable, (char *) #DOC, NULL \
-  }
+#define OBJECTGETTER(OBJ, ATTR, DOC) \
+  {(char *) #ATTR,                   \
+   (getter) pepy_##OBJ##_get_##ATTR, \
+   (setter) pepy_attr_not_writable,  \
+   (char *) #DOC,                    \
+   NULL}
 
 /* 'OPTIONAL' references the fact that these are from the Optional Header */
-#define OBJECTGETTER_OPTIONAL(ATTR, DOC)                      \
-  {                                                           \
-    (char *) #ATTR, (getter) pepy_parsed_get_optional_##ATTR, \
-        (setter) pepy_attr_not_writable, (char *) #DOC, NULL  \
-  }
+#define OBJECTGETTER_OPTIONAL(ATTR, DOC)     \
+  {(char *) #ATTR,                           \
+   (getter) pepy_parsed_get_optional_##ATTR, \
+   (setter) pepy_attr_not_writable,          \
+   (char *) #DOC,                            \
+   NULL}
 
 static PyObject *pepy_error;
 

setup.py

@@ -47,6 +47,7 @@
 
 INCLUDE_DIRS = []
 LIBRARY_DIRS = []
+LIBRARIES = []
 
 if platform.system() == "Windows":
     SOURCE_FILES.append(
@@ -64,7 +65,7 @@
     COMPILE_ARGS = ["/EHsc"]
 else:
     SOURCE_FILES.append(
-        os.path.join(here, "pe-parser-library", "src", "unicode_codecvt.cpp")
+        os.path.join(here, "pe-parser-library", "src", "unicode_libicu.cpp")
     )
     INCLUDE_DIRS += [
         "/usr/local/include",
@@ -73,8 +74,16 @@
         os.path.join(here, "pe-parser-library", "include"),
     ]
     LIBRARY_DIRS += ["/usr/lib", "/usr/local/lib"]
+    LIBRARIES += ["icuuc"]
     COMPILE_ARGS = ["-std=c++17"]
 
+    # Add Homebrew ICU paths on macOS if ICU_ROOT is set
+    if platform.system() == "Darwin":
+        icu_root = os.environ.get("ICU_ROOT")
+        if icu_root:
+            INCLUDE_DIRS.insert(0, os.path.join(icu_root, "include"))
+            LIBRARY_DIRS.insert(0, os.path.join(icu_root, "lib"))
+
 extension_mod = Extension(
     "pepy",
     define_macros=[("PEPARSE_VERSION", f'"{VERSION}"')],
@@ -83,6 +92,7 @@
     language="c++",
     include_dirs=INCLUDE_DIRS,
     library_dirs=LIBRARY_DIRS,
+    libraries=LIBRARIES,
 )
 
 setup(