[PATCH] D75453: [Driver][ARM] fix undefined behaviour when checking architecture version

[Jan Ole Hüser via Phabricator via cfe-commits]

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;
   }


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D75453.247625.patch
Type: text/x-patch
Size: 847 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20200302/3e9797fe/attachment.bin>