[loafer-19]

This crash was caused by #1359

Root Cause

Recent OpenCC configs can include a normalization pre-pass. When normalization is present, Config wraps the normal converter stages in a PipelineConverter.

PipelineConverter::GetConversionChain() previously returned nullptr because a pipeline does not have a single conversion chain. However, librime's Simplifier code inspects OpenCC through converter_->GetConversionChain()->GetConversions() and assumes the returned chain is non-null. With a pipeline-backed converter, that assumption becomes a null pointer dereference, causing the input method to crash when switching to Simplified Chinese.

There is also a separate ABI risk from the recent C++ API/interface changes: downstream C++ consumers built against the old libopencc.so.1.3 ABI can still load the new library because the SONAME did not change.

Fix

• Make PipelineConverter::GetConversionChain() return the last stage's conversion chain, matching the existing best-effort behavior of GetSegmentation().
• Update API comments to document this compatibility behavior.
• Add regression coverage for:
  • PipelineConverter::GetConversionChain()
  • configs with normalization exposing a non-null main conversion chain
• Set OPENCC_ABI_VERSION to 1.3.2 for CMake SOVERSION, so ABI-incompatible downstream C++ consumers are forced to relink instead of silently loading the wrong library.

Verification

• git diff --check
• g++ -std=c++17 -fsyntax-only ... src/ConversionChainTest.cpp
• g++ -std=c++17 -fsyntax-only ... src/ConfigTest.cpp
• Manual smoke test confirmed a converter with normalization now returns a non-null conversion chain.

Full CMake/Bazel test runs were not available in this environment because cmake and bazel are not installed.

[frankslin]

We should probably change the minor ABI version to 1.4 as there are several breaking changes introduced by the 1.3.2 release.

[loafer-19]

We should probably change the minor ABI version to 1.4 as there are several breaking changes introduced by the 1.3.2 release.

Updated:)

[frankslin]

Please correct me if I’m wrong: the simplest fix is to deprecate 1.3.2 and release 1.4.0 without API changes.

The ABI has already changed, so the clean fix is to expose that as a new ABI/API version. I don’t think we need to retrofit the new API to preserve legacy behavior, especially if that would give PipelineConverter::GetConversionChain() a best-effort meaning that is not semantically precise.

[loafer-19]

Please correct me if I’m wrong: the simplest fix is to deprecate 1.3.2 and release 1.4.0 without API changes.

The ABI has already changed, so the clean fix is to expose that as a new ABI/API version. I don’t think we need to retrofit the new API to preserve legacy behavior, especially if that would give PipelineConverter::GetConversionChain() a best-effort meaning that is not semantically precise.

that makes sense. I’ve updated this PR to only expose the ABI break by bumping the CMake SOVERSION to 1.4.

One related note: librime still seems to have a separate source-level assumption that GetConversionChain() is always non-null. With the new normalization pipeline, OpenCC may return a PipelineConverter, whose GetConversionChain() can legitimately be nullptr.

So the ABI bump should prevent old librime binaries from silently loading an incompatible OpenCC, but librime may still need a small follow-up fix to handle a null GetConversionChain() when rebuilt against the new OpenCC API.

[frankslin]

Yeah, that’s a real issue.

I think a cleaner approach may be to keep PipelineConverter semantics unchanged and introduce one more abstraction layer e.g. ConfigBasedConverter.

The idea is that PipelineConverter itself still does not have a single well-defined conversion chain, so PipelineConverter::GetConversionChain() should not need to pretend otherwise.

However, in practice, external users have been treating the converter object as a way to introspect the config. For that use case, a config-backed converter could preserve the user-facing behavior: internally it executes a two-stage pipeline, as normalization followed by the traditional conversion flow. In ConfigBasedConverter, GetConversionChain() would return the main conversion chain from the config, not an arbitrary stage of a generic Pipeline Converter.

Separately, I think we can avoid hardcoding OPENCC_ABI_VERSION as 1.4. If we release this as 1.4.0, the existing SOVERSION ${OPENCC_VERSION_MAJOR}.${OPENCC_VERSION_MINOR} logic will naturally produce 1.4.