This is the mail archive of the binutils@sourceware.org mailing list for the binutils project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [Patch mach-o/gas] support .previous


Hi Tristan,

On 5 Jan 2012, at 09:10, Tristan Gingold wrote:
+
+  new_seg = obj_mach_o_parse_section ();
+
+  if (new_seg != NULL)
+    {
+      demand_empty_rest_of_line ();
+
+      subseg_set (new_seg, 0);
+      if (now_seg != old_seg)
+	previous_section = old_seg;

What is the reason of this guard ? It seems to be that it prevents user to write code such as:


asm ("
 .section x
 […]
 .previous
");

as it would make .previous behavior undefined for the user

well, my reasoning is if the user switches to a new section that is the same as the old one - then there's no change in section... so the 'previous' one hasn't changed, because the 'current' one hasn't ... but I can see your point too...


.. happy with doing it either way, just make a choice ;) ...

(applies to each case - so the choice would be dealt with the same for each).

-
/* else, we leave the section as it was; there was a fatal error anyway. */
+ demand_empty_rest_of_line ();
}

BTW, obj_mach_o_objc_section looks furiously like obj_mach_o_known_section. Maybe we can reserve a range for objc sections and merge both implementations.

Yes, I have been aware of that all along.. reservation or perhaps I could make huge enum ... or macro-ize the process.


So far, I was consoling myself with the assertion that the compiler would sibcall and thus it wouldn't incur a big code penalty - whilst making it a bit easier to keep things separate.

(BTW, I'm not sure if we will need access to the debug section switches from elsewhere in gas, to allow dwarf debugging to be enabled for asm, it might be better to keep those separate if so).

+  segT old_seg = now_seg;
 s_comm_internal (is_local, obj_mach_o_common_parse);
+  if (now_seg != old_seg)
+    previous_section = old_seg;
+}

Humm, why does it set previous_section ?

duh... thanks for catching that.


I'll fix up and rebase according to whatever you recommend for the point above.
thanks for the review,
Iain




Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]