search

apache / arrow

Apache Arrow is a multi-language toolbox for accelerated data interchange and in-memory processing
https://arrow.apache.org/
Apache License 2.0
13.87k stars 3.38k forks source link

[CI][Archery] Archery linking should also check for undefined symbols #40018

Open raulcd opened 4 months ago

raulcd commented 4 months ago

In order to avoid issues like the one referenced below:

https://github.com/apache/arrow/blob/a0dec7f39394e619c8bdfe0eacb6ecde73a9ec12/dev/archery/archery/linking.py#L65-L75 should also check undefined symbols.

It seems that https://github.com/apache/arrow/pull/39137 is the cause of this. Sorry...

Originally posted by @kou in https://github.com/apache/arrow/issues/39919#issuecomment-1935234699

List of related issues:

danepitkin commented 3 months ago

Hey @kou , by chance is this something you could take on?

kou commented 3 months ago

I didn't have a plan to do this but I may work on this later.

BTW, I think that this is not difficult (at least for .so). We just need check symbols detected by nm XXX.so | grep '^ *U '.

danepitkin commented 3 months ago

@raulcd This should probably be something we add to 15.0.2 as a failsafe.

vibhatha commented 3 months ago

@kou something like this could work right? 

# Check for undefined symbols
result = subprocess.run(['nm', '-u', path], stdout=subprocess.PIPE, stderr=subprocess.PIPE)
undefined_symbols = result.stdout.decode('utf-8')
if undefined_symbols:
    raise DependencyError(
        f"Undefined symbols found in {dylib.path}: {undefined_symbols}"
    )

How can we test this though? cc @raulcd

kou commented 3 months ago

Wow! I didn't know the nm's -u option! But your code will not work because there are some expected undefined symbols. They are symbols provided by linked libraries. See --allow options for linked libraries:

https://github.com/apache/arrow/blob/0ce72675f4dd755b2696eb6597850b21df647bb8/ci/scripts/java_jni_manylinux_build.sh#L155-L165

We need to remove expected undefined symbols from undefined_symbols. If there are still one or more undefined symbols, we should report an error.

kou commented 3 months ago

We can test this feature by downloading arrow-dataset-X.Y.Z.jar (see https://github.com/apache/arrow/issues/39919#issuecomment-1934367934 for the URLs of them) and run the following command line:

archery linking check-dependencies \
  --allow ld-linux-aarch64 \
  --allow ld-linux-x86-64 \
  --allow libc \
  --allow libdl \
  --allow libgcc_s \
  --allow libm \
  --allow libpthread \
  --allow librt \
  --allow libstdc++ \
  --allow libz \
  --allow linux-vdso \
  arrow-dataset-X.Y.Z/x86_64/libarrow_dataset_jni.so
vibhatha commented 3 months ago

@kou thanks for this wonderful guideline. I will work on this.

vibhatha commented 3 months ago

@kou I have a draft PR: https://github.com/apache/arrow/pull/40520 But I am not very confident with the matching criterion. Locally I tested, it clears undefined symbols from 777 to 478 from the allowed symbols. I used this: https://repo1.maven.org/maven2/org/apache/arrow/arrow-dataset/15.0.0/arrow-dataset-15.0.0.jar

I think this needs more work and proper validation. Appreciate your input.

cc @raulcd @danepitkin

vibhatha commented 3 months ago

I can locally get the following output

Error: Undefined symbols found in /home/asus/sandbox/ci/arrow-dataset-15.0.0/x86_64/libarrow_dataset_jni.so:
EVP_MD_CTX_create
EVP_MD_CTX_destroy
HMAC_CTX_cleanup
HMAC_CTX_init
_ZN6google8protobuf11MessageLite15ParseFromStringERKSs
_ZN6google8protobuf16RepeatedPtrFieldISsE3AddEv
_ZN6google8protobuf16RepeatedPtrFieldISsE5ClearEv
_ZN6google8protobuf16RepeatedPtrFieldISsE7ReserveEi
_ZN6google8protobuf16RepeatedPtrFieldISsE9MergeFromERKS2_
_ZN6google8protobuf16RepeatedPtrFieldISsEC1EPNS0_5ArenaE
_ZN6google8protobuf16RepeatedPtrFieldISsEC1ERKS2_
_ZN6google8protobuf16RepeatedPtrFieldISsED1Ev
_ZN6google8protobuf2io18StringOutputStreamC1EPSs
_ZN6google8protobuf2io19EpsCopyOutputStream18WriteStringOutlineEjRKSsPh
_ZN6google8protobuf2io19EpsCopyOutputStream30WriteStringMaybeAliasedOutlineEjRKSsPh
_ZN6google8protobuf3Any9MergeImplERNS0_7MessageERKS2_
_ZN6google8protobuf4util15status_internallsERSoRKNS2_6StatusE
_ZN6google8protobuf4util18BinaryToJsonStreamEPNS1_12TypeResolverERKSsPNS0_2io19ZeroCopyInputStreamEPNS6_20ZeroCopyOutputStreamERKNS1_16JsonPrintOptionsE
_ZN6google8protobuf4util18JsonToBinaryStreamEPNS1_12TypeResolverERKSsPNS0_2io19ZeroCopyInputStreamEPNS6_20ZeroCopyOutputStreamERKNS1_16JsonParseOptionsE
_ZN6google8protobuf4util32NewTypeResolverForDescriptorPoolERKSsPKNS0_14DescriptorPoolE
_ZN6google8protobuf5Arena23AllocateAlignedWithHookEmPKSt9type_info
_ZN6google8protobuf5Arena26AllocateAlignedWithCleanupEmPKSt9type_info
_ZN6google8protobuf7Message19CopyWithSourceCheckERS1_RKS1_
_ZN6google8protobuf8internal10VerifyUTF8ENS0_20stringpiece_internal11StringPieceEPKc
_ZN6google8protobuf8internal14ArenaStringPtr12ClearToEmptyEv
_ZN6google8protobuf8internal14ArenaStringPtr3SetEOSsPNS0_5ArenaE
_ZN6google8protobuf8internal14ArenaStringPtr3SetERKSsPNS0_5ArenaE
_ZN6google8protobuf8internal14ArenaStringPtr7DestroyEv
_ZN6google8protobuf8internal14ArenaStringPtr7MutableEPNS0_5ArenaE
_ZN6google8protobuf8internal14WireFormatLite20InternalWriteMessageEiRKNS0_11MessageLiteEiPhPNS0_2io19EpsCopyOutputStreamE
_ZN6google8protobuf8internal14ZeroFieldsBase14_InternalParseEPKcPNS1_12ParseContextE
_ZN6google8protobuf8internal14ZeroFieldsBase5ClearEv
_ZN6google8protobuf8internal14ZeroFieldsBase8CopyImplERNS0_7MessageERKS3_
_ZN6google8protobuf8internal14ZeroFieldsBase9MergeImplERNS0_7MessageERKS3_
_ZN6google8protobuf8internal14ZeroFieldsBaseD2Ev
_ZN6google8protobuf8internal15ThreadSafeArena10AddCleanupEPvPFvS3_E
_ZN6google8protobuf8internal15ThreadSafeArenaD1Ev
_ZN6google8protobuf8internal17AssignDescriptorsEPFPKNS1_15DescriptorTableEvEPSt9once_flagRKNS0_8MetadataE
_ZN6google8protobuf8internal18EpsCopyInputStream12DoneFallbackEii
_ZN6google8protobuf8internal20AddDescriptorsRunnerC1EPKNS1_15DescriptorTableE
_ZN6google8protobuf8internal20RepeatedPtrFieldBase13DestroyProtosEv
_ZN6google8protobuf8internal20RepeatedPtrFieldBase18AddOutOfLineHelperEPv
_ZN6google8protobuf8internal24InlineGreedyStringParserEPSsPKcPNS1_12ParseContextE
_ZN6google8protobuf8internal26fixed_address_empty_stringE
_ZNK6google8protobuf11MessageLite17SerializeAsStringEv
_ZNK6google8protobuf11MessageLite17SerializeToStringEPSs
_ZNK6google8protobuf16RepeatedPtrFieldISsE3endEv
_ZNK6google8protobuf16RepeatedPtrFieldISsE3GetEi
_ZNK6google8protobuf16RepeatedPtrFieldISsE4sizeEv
_ZNK6google8protobuf16RepeatedPtrFieldISsE5beginEv
_ZNK6google8protobuf16RepeatedPtrFieldISsE5emptyEv
_ZNK6google8protobuf7Message11DebugStringEv
_ZNK6google8protobuf7Message11GetTypeNameEv
_ZNK6google8protobuf7Message25InitializationErrorStringEv
_ZNK6google8protobuf7Message29MaybeComputeUnknownFieldsSizeEmPNS0_8internal10CachedSizeE
_ZNK6google8protobuf8internal11AnyMetadata10InternalIsENS0_20stringpiece_internal11StringPieceE
_ZNK6google8protobuf8internal14ZeroFieldsBase12ByteSizeLongEv
_ZNK6google8protobuf8internal14ZeroFieldsBase18_InternalSerializeEPhPNS0_2io19EpsCopyOutputStreamE
_ZTIN6google8protobuf8internal14ZeroFieldsBaseE

But my nm version could be different compared to what is in the CIs

vibhatha commented 3 months ago

@kou I got a bit stuck in this PR with macOS changes. Is there an equivalent operation like ldconfig for MacOS? I am not very familiar, though finding paths in lib paths is one option.

kou commented 3 months ago

How about splitting this issue to the following sub issues?

vibhatha commented 3 months ago

Will do that @kou Thank you for the suggestion.

vibhatha commented 3 months ago

The above referenced issues have been created to track this issue.

vibhatha commented 3 months ago

@raulcd I cannot edit the issue description myself, would it be possible to add them as todo-like items in the issue description so that we can mark them complete as make progress.