Roll src/third_party/ced/src/ ba412eaaa..d127078ce (7 commits) [chromium/src : main]
Jiewei Qian (Gerrit)
Jiewei Qian added 1 comment![]( "Open in Gerrit")
if (is_linux || is_chromeos || is_android) {
defines = [ "HAVE_MEMRCHR" ]
}why this change?
---
I would expect https://github.com/google/compact_enc_det/blob/d127078cedef9c6642cbe592dacdd2292b50bb19/util/string_util.h#L44-L59 to handle this already
Related details
- Alejandro Rivera
- Archit Jain
- Joey Scarr
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Archit Jain (Gerrit)
Archit Jain added 1 comment![]( "Open in Gerrit")
if (is_linux || is_chromeos || is_android) {
defines = [ "HAVE_MEMRCHR" ]
}why this change?
---
I would expect https://github.com/google/compact_enc_det/blob/d127078cedef9c6642cbe592dacdd2292b50bb19/util/string_util.h#L44-L59 to handle this already
As part of the CED (Compact Encoding Detection) library roll, a compilation failure was identified on several platforms (Android, Linux, ChromeOS) due to a conflict in the `memrchr` function definition. The root cause is a change in the upstream CED source that restricts the detection of the system-provided `memrchr` strictly to environments where `__GLIBC__` is defined. This incorrectly triggers a fallback implementation on platforms like Android (which uses Bionic) and Fuchsia (which uses a Musl-based libc), both of which provide `memrchr` but do not define `__GLIBC__`.
The following design document outlines the strategy to resolve this by utilizing GN build configurations to correctly signal the presence of `memrchr` to the CED library, thereby bypassing the problematic fallback and restoring a green build.
**§1. Problem Statement**
- **Core Problem**: The CED roll (`ba412eaaa`..`d127078ce`) introduced a regression where `memrchr` is redefined on non-GLIBC platforms that already provide it. This leads to compilation errors such as "functions that differ only in their return type cannot be overloaded" or "redefinition of 'memrchr'".
- **Primary Deliverable**: A modified `third_party/ced/BUILD.gn` that injects the `HAVE_MEMRCHR` preprocessor define for all affected Chromium platforms.
- **Scope Boundary**:
- **In Scope**: Fixing the build configuration for Android, Linux, ChromeOS, and Fuchsia.
- **Out of Scope**: Modifying the upstream CED source code (located in `src/third_party/ced/src`), which is restricted.
**§2. Terminology**
| Term | Definition |
| :--- | :--- |
| **CED** | Compact Encoding Detection; a library for identifying text encoding (e.g., UTF-8, Shift-JIS). |
| **memrchr** | A standard GNU extension function that searches for a character in a memory buffer from right to left. |
| **Bionic** | The C library used by the Android platform. |
| **HAVE_MEMRCHR** | A preprocessor macro used by many libraries (including CED) to indicate that the system provides a `memrchr` implementation. |
**§3. Requirements**
- **Functional Requirements**:
- Suppress CED's internal `memrchr` fallback on Android, Linux, ChromeOS, and Fuchsia.
- Ensure the fallback remains active for Windows, macOS, and iOS (where `memrchr` is not available).
- **Non-Functional Requirements**:
- **Testability**: The change must be verified via `ced_unittests`.
- **Maintainability**: The solution must avoid patching upstream source to simplify future rolls.
**§4. Contract Definitions and Interfaces**
The fix relies on the existing contract in `third_party/ced/src/util/string_util.h`, which checks for the `HAVE_MEMRCHR` macro before attempting to define a fallback. By providing this macro via the compiler command line (`-DHAVE_MEMRCHR`), we fulfill the contract and prevent the redefinition.
**§4.1 Error Handling and Recovery Matrix**
| Scenario | Impact | Recovery |
| :--- | :--- | :--- |
| `HAVE_MEMRCHR` missing on Android | Build fails with redefinition error. | Add `is_android` to the GN condition. |
| `HAVE_MEMRCHR` defined on Windows | Linker fails (missing symbol). | Ensure `is_win` is excluded from the condition. |
| Upstream changes macro name | Build fails with redefinition error. | Update GN to match the new macro name. |
**§5. Architecture Overview**
The solution uses GN's `config` mechanism to propagate the `HAVE_MEMRCHR` define. Since `string_util.h` is a public header within the `ced` target, the define is placed in a `public_config` to ensure all consumers of the library (like `base:i18n`) also see the correct state, preventing ODR (One Definition Rule) violations.
**§5.1 Interaction Sequence Diagram**
```text
+----------------+ +-------------------+ +-----------------------+
| GN Build Sys | ----> | ced_config (GN) | ----> | util/string_util.h |
+----------------+ +-------------------+ +-----------------------+
| | |
| [Evaluation] | |
| If Target OS in: | |
| {Android, Linux, | [Compile Step] |
| ChromeOS, Fuchsia} | |
| | Inject -DHAVE_MEMRCHR |
+-------------------------+ |
| |
| #ifndef HAVE_MEMRCHR |
| // Skipped! |
| #endif |
+------------------------------+
Related details
- Alejandro Rivera
- Jiewei Qian
- Joey Scarr
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Archit Jain (Gerrit)
Archit Jain added 1 comment![]( "Open in Gerrit")
if (is_linux || is_chromeos || is_android) {
defines = [ "HAVE_MEMRCHR" ]
}Marked as resolved.
Related details
- Alejandro Rivera
- Jiewei Qian
- Joey Scarr
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Jiewei Qian (Gerrit)
Jiewei Qian voted Code-Review+1![]( "Open in Gerrit")
- Alejandro Rivera
- Archit Jain
- Joey Scarr
- Rachael Newitt
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |