[ELF] - Report location for div/mod by zero.

[grimar]

Summary

"division by zero" or "modulo by zero" are not
very informative errors and even probably confusing
as does not let to know that error is coming from linker script.
Patch adds location reporting.

[ruiu]

ELF/ScriptParser.cpp

815–823 ↗

(On Diff #136485)

You probably should inline these functions. They are called only once.

[grimar]

ELF/ScriptParser.cpp

815–823 ↗

(On Diff #136485)

Do you mean following ?

return [=] { return mod(L(), R(), getCurrentLocation()); };

It is not the same. Functor is called much later, and location will be unavailable,
we can't get it from inside.

[ruiu]

ELF/ScriptParser.cpp

815–823 ↗

(On Diff #136485)

No what I mean is to inline div and mod here.

[grimar]

ELF/ScriptParser.cpp

815–823 ↗

(On Diff #136485)

Ah, OK. Will update diff tomorrow with that change, thanks !

[grimar]

• Addressed review comments.

[ruiu]

LGTM

ELF/ScriptParser.cpp

808 ↗

(On Diff #136687)

nit: my preference is to use -> uint64_t instead of type casting a return value.

lld/trunk/ELF/ScriptParser.cpp

private:
   AssertCommand *readAssert();
   Expr readAssertExpr();
   Expr readConstant();
   Expr getPageSize();
 
   uint64_t readMemoryAssignment(StringRef, StringRef, StringRef);
   std::pair<uint32_t, uint32_t> readMemoryAttributes();
 
+  Expr combine(StringRef Op, Expr L, Expr R);
   Expr readExpr();
   Expr readExpr1(Expr Lhs, int MinPrec);
   StringRef readParenLiteral();
   Expr readPrimary();
   Expr readTernary(Expr Cond);
   Expr readParenExpr();
 
   // For parsing version script.

 
 static ExprValue sub(ExprValue A, ExprValue B) {
   // The distance between two symbols in sections is absolute.
   if (!A.isAbsolute() && !B.isAbsolute())
     return A.getValue() - B.getValue();
   return {A.Sec, false, A.getSectionOffset() - B.getValue(), A.Loc};
 }
 
-static ExprValue div(ExprValue A, ExprValue B) {
-  if (uint64_t BV = B.getValue())
-    return A.getValue() / BV;
-  error("division by zero");
-  return 0;
-}
-
-static ExprValue mod(ExprValue A, ExprValue B) {
-  if (uint64_t BV = B.getValue())
-    return A.getValue() % BV;
-  error("modulo by zero");
-  return 0;
-}
-
 static ExprValue bitAnd(ExprValue A, ExprValue B) {
   moveAbsRight(A, B);
   return {A.Sec, A.ForceAbsolute,
           (A.getValue() & B.getValue()) - A.getSecAddr(), A.Loc};
 }
 
 static ExprValue bitOr(ExprValue A, ExprValue B) {
   moveAbsRight(A, B);
Expr ScriptParser::readExpr() {
   // they apply different tokenization rules.
   bool Orig = InExpr;
   InExpr = true;
   Expr E = readExpr1(readPrimary(), 0);
   InExpr = Orig;
   return E;
 }
 
-static Expr combine(StringRef Op, Expr L, Expr R) {
+Expr ScriptParser::combine(StringRef Op, Expr L, Expr R) {
   if (Op == "+")
     return [=] { return add(L(), R()); };
   if (Op == "-")
     return [=] { return sub(L(), R()); };
   if (Op == "*")
     return [=] { return L().getValue() * R().getValue(); };
-  if (Op == "/")
-    return [=] { return div(L(), R()); };
-  if (Op == "%")
-    return [=] { return mod(L(), R()); };
+  if (Op == "/") {
+    std::string Loc = getCurrentLocation();
+    return [=]() -> uint64_t {
+      if (uint64_t RV = R().getValue())
+        return L().getValue() / RV;
+      error(Loc + ": division by zero");
+      return (uint64_t)0;
+    };
+  }
+  if (Op == "%") {
+    std::string Loc = getCurrentLocation();
+    return [=]() -> uint64_t {
+      if (uint64_t RV = R().getValue())
+        return L().getValue() % RV;
+      error(Loc + ": modulo by zero");
+      return (uint64_t)0;
+    };
+  }
   if (Op == "<<")
     return [=] { return L().getValue() << R().getValue(); };
   if (Op == ">>")
     return [=] { return L().getValue() >> R().getValue(); };
   if (Op == "<")
     return [=] { return L().getValue() < R().getValue(); };
   if (Op == ">")
     return [=] { return L().getValue() > R().getValue(); };

lld/trunk/test/ELF/linkerscript/operators.test

 # RUN: not ld.lld %t --script %t.script -o %t2 2>&1 | \
 # RUN:  FileCheck --check-prefix=ERREXPR %s
 # ERREXPR: malformed number: ;
 
 ## Div by zero error.
 # RUN: echo "SECTIONS { . = 1 / 0; }" > %t.script
 # RUN: not ld.lld %t --script %t.script -o %t2 2>&1 | \
 # RUN:  FileCheck --check-prefix=DIVZERO %s
-# DIVZERO: division by zero
+# DIVZERO: {{.*}}.script:1: division by zero
+
+## Mod by zero error.
+# RUN: echo "SECTIONS { . = 1 % 0; }" > %t.script
+# RUN: not ld.lld %t --script %t.script -o %t2 2>&1 | \
+# RUN:  FileCheck --check-prefix=MODZERO %s
+# MODZERO: {{.*}}.script:1: modulo by zero
 
 ## Broken ternary operator expression.
 # RUN: echo "SECTIONS { . = 1 ? 2; }" > %t.script
 # RUN: not ld.lld %t --script %t.script -o %t2 2>&1 | \
 # RUN:  FileCheck --check-prefix=TERNERR %s
 # TERNERR: : expected, but got ;