j0le created this revision.
j0le added a reviewer: compnerd.
j0le added a project: clang.
Herald added a subscriber: kristof.beyls.

Hello everyone,

this is my first patch to/for clang. I hope, I did everything right. If not, 
please tell me.

I found this bug:
If you execute the following commandline multiple times, the behavior is not 
always the same:

  clang++ --target=thumbv7em-none-windows-eabi-coff -march=armv7-m 
-mcpu=cortex-m7 -o temp.obj -c -x c++ empty.cpp

where empty.cpp is an empty file in the current working directory.

Most of the time the compilation succeeds, but sometimes clang reports this 
error:

  clang++: error: the target architecture 'thumbv7em' is not supported by the 
target 'thumbv7em-none-windows-eabi'

With these commandline arguments, the variable Version is not set by 
getAsInteger() (see diff),
because it does not parse the substring "7em" of "thumbv7em".
To get a consistent behaviour, it's enough to initialize the variable Version 
to zero.
(Zero is smaller than 7, so the comparison will be true.)
Then the command always fails with the error message seen above.

I don't know, if this is the intended behaviour.
But if it isn't, I would suggest to use the function consumeInteger() instead.
And check the return value of course.

consumeInteger() is able to get 7 from "7em".


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D75453

Files:
  clang/lib/Driver/ToolChains/Clang.cpp


Index: clang/lib/Driver/ToolChains/Clang.cpp
===================================================================
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -3531,9 +3531,10 @@
   if (Triple.isOSWindows() && (Triple.getArch() == llvm::Triple::arm ||
                                Triple.getArch() == llvm::Triple::thumb)) {
     unsigned Offset = Triple.getArch() == llvm::Triple::arm ? 4 : 6;
-    unsigned Version;
-    Triple.getArchName().substr(Offset).getAsInteger(10, Version);
-    if (Version < 7)
+    unsigned Version = 0;
+    bool Failure =
+        Triple.getArchName().substr(Offset).consumeInteger(10, Version);
+    if (Failure || Version < 7)
       D.Diag(diag::err_target_unsupported_arch) << Triple.getArchName()
                                                 << TripleStr;
   }


Index: clang/lib/Driver/ToolChains/Clang.cpp
===================================================================
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -3531,9 +3531,10 @@
   if (Triple.isOSWindows() && (Triple.getArch() == llvm::Triple::arm ||
                                Triple.getArch() == llvm::Triple::thumb)) {
     unsigned Offset = Triple.getArch() == llvm::Triple::arm ? 4 : 6;
-    unsigned Version;
-    Triple.getArchName().substr(Offset).getAsInteger(10, Version);
-    if (Version < 7)
+    unsigned Version = 0;
+    bool Failure =
+        Triple.getArchName().substr(Offset).consumeInteger(10, Version);
+    if (Failure || Version < 7)
       D.Diag(diag::err_target_unsupported_arch) << Triple.getArchName()
                                                 << TripleStr;
   }
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to