[m-rev.] for review: fix bug #196: binary compatiblity checks in C grade don't work

Julien Fischer jfischer at opturion.com
Sun Jul 17 13:49:49 AEST 2016


Hi Zoltan,

On Thu, 14 Jul 2016, Zoltan Somogyi wrote:
> On Fri, 15 Jul 2016 01:16:25 +1000 (AEST), Julien Fischer <jfischer at opturion.com> wrote:
>> Is anyone intending to review this one?  In the absence of any comments
>> I'll commit it later today.
>
> I thought I already reviewed the idea, and said that I would prefer
> to fix the problem by exporting a function that refers to the value
> of MR_GRADE_VAR. I think this would be more robust in the face
> of compiler optimizations, though it does have the slight downside
> of adding one symbol per module to the executable's symbol table.

The following diff implements the above.

Julien.

---------------

Fix bug #196: binary compatibility checks in C grades do not work.

These have been broken since 2011 because GCC and clang (at least) began
optimising away the MR_grade variable we emit at the end of each generated .c
file.  The fix is to replace the MR_grade variable with an exported function
that references MR_GRADE_VAR in each generated .c file.

compiler/llds_out_file.m:
compiler/mlds_to_c.m:
util/mkinit.c:
      In each generated .c file, emit an exported function that references
      MR_GRADE_VAR.

diff --git a/compiler/llds_out_file.m b/compiler/llds_out_file.m
index f437ce4..e02410e 100644
--- a/compiler/llds_out_file.m
+++ b/compiler/llds_out_file.m
@@ -482,6 +482,10 @@ output_c_module_init_list(Info, ModuleName, AnnotatedModules, RttiDatas,
          io.write_string("required_final(void);\n", !IO)
      ),

+    io.write_string("const char *", !IO),
+    output_init_name(ModuleName, !IO),
+    io.write_string("grade_check(void);\n", !IO),
+
      io.write_string("\n", !IO),

      io.write_string("void ", !IO),
@@ -609,14 +613,19 @@ output_c_module_init_list(Info, ModuleName, AnnotatedModules, RttiDatas,
          io.write_string("required_final(void)\n", !IO),
          io.write_string("{\n", !IO),
          output_required_init_or_final_calls(FinalPredNames, !IO),
-        io.write_string("}\n", !IO)
+        io.write_string("}\n", !IO),
+        io.nl(!IO)
      ),

      io.write_string(
-        "/* ensure everything is compiled with the same grade */\n",
+        "// Ensure everything is compiled with the same grade.\n",
          !IO),
-    io.write_string(
-        "static const void *const MR_grade = &MR_GRADE_VAR;\n", !IO).
+    io.write_string("const char *", !IO),
+    output_init_name(ModuleName, !IO),
+    io.write_string("grade_check(void)\n", !IO),
+    io.write_string("{\n", !IO),
+    io.write_string("    return &MR_GRADE_VAR;\n", !IO),
+    io.write_string("}\n", !IO).

  :- pred module_defines_label_with_layout(llds_out_info::in,
      annotated_c_module::in) is semidet.
diff --git a/compiler/mlds_to_c.m b/compiler/mlds_to_c.m
index eeb570c..9a9608d 100644
--- a/compiler/mlds_to_c.m
+++ b/compiler/mlds_to_c.m
@@ -455,7 +455,7 @@ mlds_output_src_file(Opts, Indent, MLDS, !IO) :-
      mlds_output_init_fn_defns(Opts, MLDS_ModuleName, FuncDefns,
          TypeCtorInfoDefns, AllocSites, InitPreds, FinalPreds, !IO),
      io.nl(!IO),
-    mlds_output_grade_var(!IO),
+    mlds_output_grade_check_fn_defn(MLDS_ModuleName, !IO),
      io.nl(!IO),
      mlds_output_src_end(Indent, ModuleName, !IO).

@@ -672,15 +672,16 @@ mlds_output_auto_gen_comment(Opts, ModuleName, !IO) :-
      % file gets compiled with. This ensures that we don't try to link objects
      % files compiled in different grades.
      %
-:- pred mlds_output_grade_var(io::di, io::uo) is det.
+:- pred mlds_output_grade_check_fn_defn(mlds_module_name::in, io::di, io::uo) is det.

-mlds_output_grade_var(!IO) :-
+mlds_output_grade_check_fn_defn(ModuleName, !IO) :-
      io.write_string(
-        "/* ensure everything is compiled with the same grade */\n",
+        "// Ensure everything is compiled with the same grade.\n",
          !IO),
-    io.write_string(
-        "static const void *const MR_grade = &MR_GRADE_VAR;\n",
-        !IO).
+    output_grade_check_fn_name(ModuleName, !IO),
+    io.write_string("\n{\n", !IO),
+    io.write_string("    return &MR_GRADE_VAR;\n", !IO),
+    io.write_string("}\n", !IO).

      % Get the foreign code for C.
      %
@@ -729,7 +730,9 @@ mlds_output_init_fn_decls(ModuleName, InitPreds, FinalPreds, !IO) :-
          FinalPreds = [_ | _],
          output_required_fn_name(ModuleName, "required_final", !IO),
          io.write_string(";\n", !IO)
-    ).
+    ),
+    output_grade_check_fn_name(ModuleName, !IO),
+    io.write_string(";\n", !IO).

  :- pred mlds_output_init_fn_defns(mlds_to_c_opts::in, mlds_module_name::in,
      list(mlds_defn)::in, list(mlds_defn)::in,
@@ -847,6 +850,20 @@ output_required_fn_name(ModuleName, Suffix, !IO) :-
      io.write_string(Suffix, !IO),
      io.write_string("(void)", !IO).

+:- pred output_grade_check_fn_name(mlds_module_name::in,
+    io::di, io::uo) is det.
+
+output_grade_check_fn_name(ModuleName, !IO) :-
+    ModuleNameString0 = sym_name_mangle(
+        mlds_module_name_to_sym_name(ModuleName)),
+    ( if string.prefix(ModuleNameString0, "mercury__") then
+        ModuleNameString = ModuleNameString0
+    else
+        ModuleNameString = "mercury__" ++ ModuleNameString0
+    ),
+    io.format("const char *%s__grade_check(void)",
+        [s(ModuleNameString)], !IO).
+
      % Generate calls to MR_init_entry() for the specified functions.
      %
  :- pred mlds_output_calls_to_init_entry(mlds_module_name::in,
diff --git a/util/mkinit.c b/util/mkinit.c
index 0e15a60..cbaa5b6 100644
--- a/util/mkinit.c
+++ b/util/mkinit.c
@@ -619,9 +619,12 @@ static const char mercury_main_func[] =
      "\n"
      ;

-static const char mercury_grade_var[] =
-    "/* ensure that everything gets compiled in the same grade */\n"
-    "static const void *const MR_grade = &MR_GRADE_VAR;\n"
+static const char mercury_grade_check_func[] =
+    "// Ensure that everything gets compiled in the same grade.\n"
+    "const char *mercury_init_grade_check(void)\n"
+    "{\n"
+    "    return &MR_GRADE_VAR;\n"
+    "}\n"
      "\n"
      ;

@@ -1201,7 +1204,7 @@ output_main(void)
          fputs(mercury_main_func, stdout);
      }

-    fputs(mercury_grade_var, stdout);
+    fputs(mercury_grade_check_func, stdout);

      if (output_main_func) {
          fputs(main_func, stdout);


More information about the reviews mailing list