Merge lp:~gue5t/midori/64-bit-vala-decl into lp:midori

[gue5t gue5t]

Ensure vala knows the prototypes of functions it calls, fixing pointer truncation in tests

I was getting segfaults when running the 'actions' test despite code that looked perfectly sane. When I investigated in gdb, everything looked good until the function returned--at which point the pointer was getting manged! In the disassembly, I saw a "zero-extend" instruction being used on the pointer, which made no sense. Then I realized that the function being called in Vala, 263midori_view_get_page_context_action, was not declared in a header. This means that in the generated C, the function was implicitly declared. In C, implicit declarations have "int" return type, which is 4 bytes instead of 8 on x86_64. Thus the pointer was converted to an int and back to a pointer, shaving off the top 4 bytes.

Adding a declaration in the header stopped the segfaults. Can we get valac to ask the C compiler to disallow implicit declarations?

[Cris Dywan (kalikiana)]

How about adding -Werror=implicit-function-declaration to VALA_CFLAGS in CmakeLists.txt?

It seems a little odd to me to be asserting the name of the menhu, but I'm guressing you're doing this to dare the pointer to get corrupted... so maybe a sensible resort

review: Needs Information

[lp:~gue5t/midori/64-bit-vala-decl]

6983. By gue5t <email address hidden> on 2015-08-10

Forbid implicit declarations in valac-generated C and enable non-superfluous warnings; declare midori_search_action_token_for_uri

[Cris Dywan (kalikiana)]

I'm getting a ton of warnings building this:

./katze/midori-paths.vala:333:10 warning: assignment from incompatible pointer type
 if (strcmp (Environment.get_variable ("MIDORI_DEBUG"), "paths") == 0) {
 ^

cc1: warning: unrecognized command line option "-Wno-discarded-qualifiers"
cc1: warning: unrecognized command line option "-Wno-incompatible-pointer-types"

review: Needs Fixing

[gue5t gue5t (gue5t)]

Ah, looks like those are new in GCC 5. I'll have to make them conditional on that compiler+version, then.

[lp:~gue5t/midori/64-bit-vala-decl]

6984. By gue5t <email address hidden> on 2015-08-15

Only enable warnings for Vala with recent versions of GCC and Clang

[gue5t gue5t (gue5t)]

I've changed it to only enable warnings with recent versions of GCC and Clang; other versions/compilers will just have no warnings from Valac-generated C.

[Paweł Forysiuk (tuxator)]

review: Approve

=== modified file 'CMakeLists.txt'
--- CMakeLists.txt	2015-07-19 22:25:09 +0000
+++ CMakeLists.txt	2015-08-15 23:41:09 +0000
@@ -255,7 +255,19 @@
 endif ()
 
 # Explicitly add -fPIC for older toolchains
-set(VALA_CFLAGS "-w -g -fPIC")
+set(VALA_CFLAGS "-g -fPIC")
+
+# With compiler versions that can, enable exactly the non-spurious warnings
+# in Vala-generated C, otherwise disable warnings
+if ((CMAKE_C_COMPILER_ID STREQUAL "GNU" AND CMAKE_C_COMPILER_VERSION VERSION_GREATER "5.0.0")
+    OR (CMAKE_C_COMPILER_ID STREQUAL "Clang" AND CMAKE_C_COMPILER_VERSION VERSION_GREATER "3.0.0"))
+    set(VALA_CFLAGS "${VALA_CFLAGS} -Werror=implicit-function-declaration")
+    set(VALA_CFLAGS "${VALA_CFLAGS} -Wno-incompatible-pointer-types")
+    set(VALA_CFLAGS "${VALA_CFLAGS} -Wno-discarded-qualifiers")
+    set(VALA_CFLAGS "${VALA_CFLAGS} -Wno-deprecated-declarations")
+else ()
+    set(VALA_CFLAGS "${VALA_CFLAGS} -w")
+endif ()
 
 set(LIBMIDORI "${CMAKE_PROJECT_NAME}-core")
 
 
=== modified file 'midori/midori-searchaction.h'
--- midori/midori-searchaction.h	2014-07-24 11:48:46 +0000
+++ midori/midori-searchaction.h	2015-08-15 23:41:09 +0000
@@ -63,6 +63,9 @@
 midori_search_action_set_default_item      (MidoriSearchAction* search_action,
                                             KatzeItem*          item);
 
+gchar*
+midori_search_action_token_for_uri         (const gchar*        uri);
+
 GtkWidget*
 midori_search_action_get_dialog            (MidoriSearchAction* search_action);
 
 
=== modified file 'midori/midori-view.h'
--- midori/midori-view.h	2014-07-26 22:57:50 +0000
+++ midori/midori-view.h	2015-08-15 23:41:09 +0000
@@ -259,6 +259,10 @@
                                         GTlsCertificateFlags* tls_flags,
                                         gchar**               hostname);
 
+MidoriContextAction*
+midori_view_get_page_context_action (MidoriView*          view,
+                                     WebKitHitTestResult* hit_test_result);
+
 G_END_DECLS
 
 #endif /* __MIDORI_VIEW_H__ */
 
=== modified file 'tests/actions.vala'
--- tests/actions.vala	2013-08-02 23:45:16 +0000
+++ tests/actions.vala	2015-08-15 23:41:09 +0000
@@ -20,13 +20,17 @@
 
     var hit_test_result = Object.new (typeof (WebKit.HitTestResult), "context", WebKit.HitTestResultContext.DOCUMENT) as WebKit.HitTestResult;
     var menu = view.get_page_context_action (hit_test_result);
+    assert (menu != null);
     assert (menu.name == "PageContextMenu");
     assert (menu.get_by_name ("Back") != null);
 
 #if !HAVE_WEBKIT2
-    hit_test_result = Object.new (typeof (WebKit.HitTestResult), "context", WebKit.HitTestResultContext.EDITABLE) as WebKit.HitTestResult;
-    menu = view.get_page_context_action (hit_test_result);
-    var copy = menu.get_by_name ("Copy");
+    var hit_test_result2 = Object.new (typeof (WebKit.HitTestResult), "context", WebKit.HitTestResultContext.EDITABLE) as WebKit.HitTestResult;
+    var menu2 = view.get_page_context_action (hit_test_result2);
+    assert (menu2 != null);
+    assert (menu2.name == "PageContextMenu");
+    var copy = menu2.get_by_name ("Copy");
+    assert (copy != null);
     assert (!copy.sensitive);
     assert (view.web_view.search_text ("flat", true, false, false));
     menu = view.get_page_context_action (hit_test_result);