Midori Web Browser

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

  1. 64-bit-vala-decl
  2. Merge into trunk
Proposed by gue5t gue5t
Status: Merged
Approved by: Paweł Forysiuk
Approved revision: 6984
Merged at revision: 7023
Proposed branch: lp://qastaging/~gue5t/midori/64-bit-vala-decl
Merge into: lp://qastaging/midori
Diff against target: 78 lines (+27/-4)
4 files modified
CMakeLists.txt (+13/-1)
midori/midori-searchaction.h (+3/-0)
midori/midori-view.h (+4/-0)
tests/actions.vala (+7/-3)
To merge this branch: bzr merge lp://qastaging/~gue5t/midori/64-bit-vala-decl
Reviewer Review Type Date Requested Status
Paweł Forysiuk Approve
Cris Dywan Needs Fixing
Review via email: mp+267466@code.qastaging.launchpad.net

Commit message

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

Description of the change

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?

To post a comment you must log in.
Revision history for this message
Cris Dywan (kalikiana) wrote :

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://qastaging/~gue5t/midori/64-bit-vala-decl updated
6983. By gue5t <email address hidden>

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

Revision history for this message
Cris Dywan (kalikiana) wrote :

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
Revision history for this message
gue5t gue5t (gue5t) wrote :

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

lp://qastaging/~gue5t/midori/64-bit-vala-decl updated
6984. By gue5t <email address hidden>

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

Revision history for this message
gue5t gue5t (gue5t) wrote :

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.

Revision history for this message
Paweł Forysiuk (tuxator) :
review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
The diff is not available at this time. You can reload the page or download it.

Subscribers

People subscribed via source and target branches

to all changes: