[lld] r236726 - [ARM] llvm_unreachable => make_*_reloc_error in group relocs

[Denis Protivensky]

Once again: tests covering *missing* functionality are useless.
It's the same as adding tests to missing relocations and catching error output informing that relocations are missing.

When and if someone adds logic to handle negative offsets for group relocations, they would add proper test cases.
For now on the linker will inform users that it can't perform handling of group relocations with negative offsets, which is perfectly enough for diagnostics.

Thanks,
  Denis.

On 05/22/2015 10:10 AM, Filipe Cabecinhas wrote:
It's adding tests for actual functionality:
  If the offset is negative, error out with "blah".

Regards,

  Filipe

On Thursday, May 21, 2015, Denis Protivensky <dprotivensky at accesssoftek.com<mailto:dprotivensky at accesssoftek.com>> wrote:
Yes, I know that. It's not error handling, but rather an indication of missing functionality.
I consider adding tests for not implemented functionality to be excessive.

On 05/22/2015 02:52 AM, Rafael Espíndola wrote:

It is good practice to include a test case when adding error handling.

On May 7, 2015 9:48 AM, "Denis Protivensky" <dprotivensky at accesssoftek.com<javascript:_e(%7B%7D,'cvml','dprotivensky at accesssoftek.com');>> wrote:
Author: denis-protivensky
Date: Thu May  7 08:41:44 2015
New Revision: 236726

URL: http://llvm.org/viewvc/llvm-project?rev=236726&view=rev<https://urldefense.proofpoint.com/v2/url?u=http-3A__llvm.org_viewvc_llvm-2Dproject-3Frev-3D236726-26view-3Drev&d=AwMGaQ&c=8hUWFZcy2Z-Za5rBPlktOQ&r=mQ4LZ2PUj9hpadE3cDHZnIdEwhEBrbAstXeMaFoB9tg&m=iixKjXcsENgPMCwJibZbCizoIK42E3-Z0gcTZTBaXT0&s=Zh9HDU2dYjvwOEHpIujW76DGzDgMjWcF5EHWbbutZGc&e=>
Log:
[ARM] llvm_unreachable => make_*_reloc_error in group relocs

Modified:
    lld/trunk/lib/ReaderWriter/ELF/ARM/ARMRelocationHandler.cpp

Modified: lld/trunk/lib/ReaderWriter/ELF/ARM/ARMRelocationHandler.cpp
URL: http://llvm.org/viewvc/llvm-project/lld/trunk/lib/ReaderWriter/ELF/ARM/ARMRelocationHandler.cpp?rev=236726&r1=236725&r2=236726&view=diff<https://urldefense.proofpoint.com/v2/url?u=http-3A__llvm.org_viewvc_llvm-2Dproject_lld_trunk_lib_ReaderWriter_ELF_ARM_ARMRelocationHandler.cpp-3Frev-3D236726-26r1-3D236725-26r2-3D236726-26view-3Ddiff&d=AwMGaQ&c=8hUWFZcy2Z-Za5rBPlktOQ&r=mQ4LZ2PUj9hpadE3cDHZnIdEwhEBrbAstXeMaFoB9tg&m=iixKjXcsENgPMCwJibZbCizoIK42E3-Z0gcTZTBaXT0&s=Q4c3WV1ZryTA_gWgPsQ3tpIeDZpZAzpx4aCkNiL3Axc&e=>
==============================================================================
--- lld/trunk/lib/ReaderWriter/ELF/ARM/ARMRelocationHandler.cpp (original)
+++ lld/trunk/lib/ReaderWriter/ELF/ARM/ARMRelocationHandler.cpp Thu May  7 08:41:44 2015
@@ -112,6 +112,11 @@ static Reference::Addend readAddend(cons
   }
 }

+static inline std::error_code make_unsupported_range_group_reloc_error() {
+  return make_dynamic_error_code(
+      "Negative offsets for group relocations are not implemented");
+}
+
 static inline std::error_code applyArmReloc(uint8_t *location, uint32_t result,
                                             uint32_t mask = 0xFFFFFFFF) {
   assert(!(result & ~mask));
@@ -475,10 +480,8 @@ static std::error_code relocR_ARM_ALU_PC
 static std::error_code relocR_ARM_ALU_PC_G0_NC(uint8_t *location, uint64_t P,
                                                uint64_t S, int64_t A) {
   int32_t result = (int32_t)(S + A - P);
-
   if (result < 0)
-    llvm_unreachable(
-        "Negative offsets for group relocations has not been implemented");
+    return make_unsupported_range_group_reloc_error();

   DEBUG(llvm::dbgs() << "\t\tHandle " << LLVM_FUNCTION_NAME << " -";
         llvm::dbgs() << " S: 0x" << Twine::utohexstr(S);
@@ -494,10 +497,8 @@ static std::error_code relocR_ARM_ALU_PC
 static std::error_code relocR_ARM_ALU_PC_G1_NC(uint8_t *location, uint64_t P,
                                                uint64_t S, int64_t A) {
   int32_t result = (int32_t)(S + A - P);
-
   if (result < 0)
-    llvm_unreachable(
-        "Negative offsets for group relocations has not been implemented");
+    return make_unsupported_range_group_reloc_error();

   DEBUG(llvm::dbgs() << "\t\tHandle " << LLVM_FUNCTION_NAME << " -";
         llvm::dbgs() << " S: 0x" << Twine::utohexstr(S);
@@ -513,10 +514,8 @@ static std::error_code relocR_ARM_ALU_PC
 static std::error_code relocR_ARM_LDR_PC_G2(uint8_t *location, uint64_t P,
                                             uint64_t S, int64_t A) {
   int32_t result = (int32_t)(S + A - P);
-
   if (result < 0)
-    llvm_unreachable(
-        "Negative offsets for group relocations has not been implemented");
+    return make_unsupported_range_group_reloc_error();

   DEBUG(llvm::dbgs() << "\t\tHandle " << LLVM_FUNCTION_NAME << " -";
         llvm::dbgs() << " S: 0x" << Twine::utohexstr(S);


_______________________________________________
llvm-commits mailing list
llvm-commits at cs.uiuc.edu<javascript:_e(%7B%7D,'cvml','llvm-commits at cs.uiuc.edu');>
http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits



--
  F


-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20150522/3e49cce5/attachment.html>