[PATCH] ARM unwind directive parser, assembler, and testcases.

Jim Grosbach grosbach at apple.com
Tue Apr 23 12:32:03 PDT 2013



================
Comment at: lib/Target/ARM/MCTargetDesc/ARMELFStreamer.cpp:328
@@ -326,2 +327,3 @@
 void ARMELFStreamer::EmitFnStart() {
-  assert(FnStart == 0);
+  if (FnStart)
+    report_fatal_error("Previous .fnstart directive is not closed.");
----------------
Diagnostics like this really should be part of the asmparser, not the streamer. Here, we don't have any source location information to do anything useful. By this point, an assert() is correct.

Ditto for the similar changes below.

================
Comment at: lib/Target/ARM/MCTargetDesc/ARMELFStreamer.cpp:407
@@ -402,8 +406,3 @@
 
-  // Emit Personality
-  assert(Personality && ".personality directive must preceed .handlerdata");
-
-  const MCSymbolRefExpr *PersonalityRef =
-    MCSymbolRefExpr::Create(Personality,
-                            MCSymbolRefExpr::VK_ARM_PREL31,
-                            getContext());
+  // Emit personality
+  if (Personality) {
----------------
Is it ever correct to not have a personality function? I would think not, and the assert() was correct.

================
Comment at: lib/Target/ARM/MCTargetDesc/ARMELFStreamer.cpp:434
@@ -428,1 +433,3 @@
+  if (SPOffset != 0)
+    report_fatal_error("Current implementation assumes .setfp precedes .pad");
 
----------------
This should be in the parser as noted above. That said, can you elaborate on why the implementation assumes this? Should it? Basically, I don't understand what this message is trying to say, when it would be encountered, etc..


http://llvm-reviews.chandlerc.com/D692

BRANCH
  add-arm-unwind-directives-squashed

ARCANIST PROJECT
  llvm



More information about the llvm-commits mailing list