Cross validate Rust ABI with C ABI. #558

ghost · 2018-03-01T15:12:12Z

This is still work in progress. Just in case you have any comments already,
or want to test it out.

Note: I am not using cc and pkg-config crates because they are ill-suited for
usage outside build.rs. For example, cc expects various env variables to be
preconfigured and errors out otherwise, nor does it supports building
executables. Pkg-config prints out cargo specific things on stdout as a side
effect of its usage, and it doesn't even provide access to complete cflags.

EPashkin · 2018-03-01T16:19:23Z

Glib test buils but failed

running 1 test
test test_cross_validate_abi_with_c ... FAILED

failures:

---- test_cross_validate_abi_with_c stdout ----
	thread 'test_cross_validate_abi_with_c' panicked at 'called `Result::unwrap()` on an `Err` value: ParseIntError { kind: InvalidDigit }', libcore\result.rs:945:5
note: Some details are omitted, run with `RUST_BACKTRACE=full` for a verbose backtrace.
stack backtrace:
   0: std::sys_common::backtrace::_print
             at libstd\sys\windows\backtrace/mod.rs:65
             at libstd\sys_common/backtrace.rs:71
   1: std::panicking::default_hook::{{closure}}
             at libstd\sys_common/backtrace.rs:59
             at libstd/panicking.rs:206
   2: std::panicking::default_hook
             at libstd/panicking.rs:216
   3: std::panicking::rust_panic_with_hook
             at libstd/panicking.rs:400
   4: std::panicking::begin_panic_fmt
             at libstd/panicking.rs:347
   5: rust_begin_unwind
             at libstd/panicking.rs:323
   6: core::panicking::panic_fmt
             at libcore/panicking.rs:71
   7: core::result::unwrap_failed
             at C:\projects\rust\src\libcore/macros.rs:23
   8: <core::result::Result<T, E>>::unwrap
             at C:\projects\rust\src\libcore/result.rs:782
   9: abi::get_c_abi
             at tests/abi.rs:140
  10: abi::test_cross_validate_abi_with_c
             at tests/abi.rs:95
  11: abi::__test::TESTS::{{closure}}
             at tests/abi.rs:84
  12: core::ops::function::FnOnce::call_once
             at C:\projects\rust\src\libcore\ops/function.rs:223
  13: <F as alloc::boxed::FnBox<A>>::call_box
             at libtest/lib.rs:1471
             at C:\projects\rust\src\libcore\ops/function.rs:223
             at C:\projects\rust\src\liballoc/boxed.rs:783
  14: _rust_maybe_catch_panic
             at libpanic_unwind/lib.rs:102

It normal that generated ABI executable print "\nzu\nzu" ?

ghost · 2018-03-01T17:23:19Z

Not expected at all. If you are running this on Windows, I am surprised it got
that far. Currently %zu is used to print size_t, but this requires at least C99.
Changed to G_GSIZE_FORMAT from glib, it should be more portable.

I wonder how exactly do you run this on Windows, under msys2?

EPashkin · 2018-03-01T17:37:46Z

Msys2x64 with gcc from it.
Nothing changed after usage G_GSIZE_FORMAT :(

EPashkin · 2018-03-01T17:38:30Z

gcc.EXE (Rev1, Built by MSYS2 project) 7.3.0

ghost · 2018-03-01T19:47:21Z

There is really nice writeup why it doesn't work: https://stackoverflow.com/questions/10678124/mingw-gcc-unknown-conversion-type-character-h-snprintf#10680635. As a workaround one can use following before running tests export CPPFLAGS=-D__USE_MINGW_ANSI_STDIO=1. I will have to think about proper long-term solution that would work out of the box.

On Windows the glib-sys test fails with following:

ABI mismatch for GPid
Rust: ABI { size: 4, alignment: 4 }
C:    ABI { size: 8, alignment: 8 }
ABI mismatch for GPollFD
Rust: ABI { size: 8, alignment: 4 }
C:    ABI { size: 16, alignment: 8 }

EPashkin · 2018-03-01T19:59:01Z

Really error for GPid
pub type GPid = c_int; va typedef void * GPid for Windows64, can't find linux definition.

GPollFD seems also have different sizes

pub struct GPollFD {
    pub fd: c_int,
    pub events: c_ushort,
    pub revents: c_ushort,
}

vs

struct _GPollFD
{
#if defined (G_OS_WIN32) && GLIB_SIZEOF_VOID_P == 8
#ifndef __GTK_DOC_IGNORE__
  gint64	fd;
#endif
#else
  gint		fd;
#endif
  gushort 	events;
  gushort 	revents;
};

EPashkin · 2018-03-01T20:09:42Z

Confirming that set CPPFLAGS=-D__USE_MINGW_ANSI_STDIO=1 helps on tests
and results same 2 errors.

ghost · 2018-03-02T15:50:20Z

Updated template code so that _D__USE_MINGW_ANSI_STDIO=1 should no longer be necessary on Windows.
Test codegen now understands sys ignores.
It works best with following fixes to gir files Add missing includes in GLib, GObject, GdkPixbuf, Gtk and Pango. gir-files#16.

EPashkin · 2018-03-02T16:24:35Z

Still don't work without D__USE_MINGW_ANSI_STDIO=1
Strange as it present in comand line

        compilation command "cc" "-D__USE_MINGW_ANSI_STDIO=1" "-mms-bitfields" "-ID:/P/msys64/mingw64/include/glib-2.0" "-ID:/P/msys64/mingw64
/lib/glib-2.0/include" "-ID:/P/msys64/mingw64/include" "-DABI_TEST_TYPE=GTypeCValue" "tests/abi.c" "-oabi" failed, exit code: 1

GIO don't compiled until i leave only #include <gio/gio.h> form <gio/*>
Glib has same 2 errors.
GObject failed on GTypeCValue

tests/abi.c: In function 'main':
<command-line>:0:15: error: invalid application of 'sizeof' to incomplete type 'GTypeCValue {aka union _GTypeCValue}'
tests/abi.c:8:31: note: in expansion of macro 'ABI_TEST_TYPE'
   printf("%zu\n%zu\n", sizeof(ABI_TEST_TYPE), alignof(ABI_TEST_TYPE));
                               ^~~~~~~~~~~~~

Gtk too:

tests/abi.c: In function 'main':
<command-line>:0:15: error: 'GtkPlug' undeclared (first use in this function); did you mean 'AtkPlug'?
tests/abi.c:9:31: note: in expansion of macro 'ABI_TEST_TYPE'
   printf("%zu\n%zu\n", sizeof(ABI_TEST_TYPE), alignof(ABI_TEST_TYPE));
                               ^~~~~~~~~~~~~
<command-line>:0:15: note: each undeclared identifier is reported only once for each function it appears in
tests/abi.c:9:31: note: in expansion of macro 'ABI_TEST_TYPE'
   printf("%zu\n%zu\n", sizeof(ABI_TEST_TYPE), alignof(ABI_TEST_TYPE));
                               ^~~~~~~~~~~~~
tests/abi.c: In function 'main':
<command-line>:0:15: error: 'GtkSocket' undeclared (first use in this function); did you mean 'AtkSocket'?
tests/abi.c:9:31: note: in expansion of macro 'ABI_TEST_TYPE'
   printf("%zu\n%zu\n", sizeof(ABI_TEST_TYPE), alignof(ABI_TEST_TYPE));
                               ^~~~~~~~~~~~~
<command-line>:0:15: note: each undeclared identifier is reported only once for each function it appears in
tests/abi.c:9:31: note: in expansion of macro 'ABI_TEST_TYPE'
   printf("%zu\n%zu\n", sizeof(ABI_TEST_TYPE), alignof(ABI_TEST_TYPE));
                               ^~~~~~~~~~~~~
tests/abi.c: In function 'main':
<command-line>:0:15: error: 'GtkStackAccessible' undeclared (first use in this function); did you mean 'GtkScaleAccessible'?
tests/abi.c:9:31: note: in expansion of macro 'ABI_TEST_TYPE'
   printf("%zu\n%zu\n", sizeof(ABI_TEST_TYPE), alignof(ABI_TEST_TYPE));
                               ^~~~~~~~~~~~~
<command-line>:0:15: note: each undeclared identifier is reported only once for each function it appears in
tests/abi.c:9:31: note: in expansion of macro 'ABI_TEST_TYPE'
   printf("%zu\n%zu\n", sizeof(ABI_TEST_TYPE), alignof(ABI_TEST_TYPE));
                               ^~~~~~~~~~~~~
tests/abi.c: In function 'main':
<command-line>:0:15: error: 'GtkStackAccessibleClass' undeclared (first use in this function); did you mean 'GtkScaleAccessibleClass'?
tests/abi.c:9:31: note: in expansion of macro 'ABI_TEST_TYPE'
   printf("%zu\n%zu\n", sizeof(ABI_TEST_TYPE), alignof(ABI_TEST_TYPE));
                               ^~~~~~~~~~~~~
<command-line>:0:15: note: each undeclared identifier is reported only once for each function it appears in
tests/abi.c:9:31: note: in expansion of macro 'ABI_TEST_TYPE'
   printf("%zu\n%zu\n", sizeof(ABI_TEST_TYPE), alignof(ABI_TEST_TYPE));

Pango also have many errors.
Pango-cairo just don't have types to check

ghost · 2018-03-02T16:49:59Z

Thanks for testing @EPashkin.

GTypeCValue needs additional include gobject/gvaluecollector.h, I will update gir-files.
Compiling Gio is problematic as it currently contains unix specific headers.
Should we remove them from gir and ignore non-portable types, if any?
GtkPlug, GtkSocket is X11 specific. Ignore them in sys crates?
GtkStackAccessible is a bug in gtk headers, it should be included as part of gtk/gtk-a11y.h, but currently isn't.
Neither can it be included directly, because it contains special guard. I will report this upstream.

AFAIR I was compiling pango with additional define: -DPANGO_ENABLE_ENGINE,
it reduced the amount of errors in pango.

EPashkin · 2018-03-02T17:25:36Z

IMHO no help for Gio on windows. How you plan detect non-portable types?
GtkPlug, GtkSocket used in Gtk with cfg_condition = "not(windows)". Bad that sys don't use this config now.

ghost · 2018-03-03T09:33:10Z

Latest results for Windows when using MSYS2 MinGW 64-bit with following flags
for pango: export CFLAGS='-DPANGO_ENABLE_BACKEND -DPANGO_ENABLE_ENGINE'

atk-sys - OK
gdk-pixbuf-sys - OK
gdk-sys - OK
glib-sys - everything compiles; ABI mismatch for GPid, GPollFd
gobject-sys - OK
pango-sys - PangoScriptForLang undeclared, everything else OK
secret-sys - OK

EPashkin · 2018-03-03T10:01:33Z

Pango still failed for me

running 1 test
cc: error: '': No such file or directory
cc: error: '-DPANGO_ENABLE_BACKEND: No such file or directory
test test_cross_validate_abi_with_c ... FAILED

failures:

---- test_cross_validate_abi_with_c stdout ----
        thread 'test_cross_validate_abi_with_c' panicked at 'C ABI for char: StringError("compilation command \"cc\" \"\\\'\\\'\" \"\\\'-DPANG
O_ENABLE_BACKEND\" \"-DPANGO_ENABLE_ENGINE\\\'\" \"-mms-bitfields\" \"-ID:/P/msys64/mingw64/include/pango-1.0\" \"-ID:/P/msys64/mingw64/includ
e/glib-2.0\" \"-ID:/P/msys64/mingw64/lib/glib-2.0/include\" \"-ID:/P/msys64/mingw64/include\" \"-Wno-deprecated-declarations\" \"-DABI_TEST_TY
PE=char\" \"tests/abi.c\" \"-oabi\" failed, exit code: 1")', libcore\result.rs:945:5
note: Run with `RUST_BACKTRACE=1` for a backtrace.

ghost · 2018-03-03T11:55:48Z

It looks like a problem with exporting environment variable.
Value of environment variable CFLAGS should be:

-DPANGO_ENABLE_BACKEND -DPANGO_ENABLE_ENGINE

instead, it is:

'-DPANGO_ENABLE_BACKEND -DPANGO_ENABLE_ENGINE'

Are you using cmd.exe instead of posix shell?
Escaping rules are somewhat different there.

EPashkin · 2018-03-03T12:06:22Z

I build under cmd.exe.
In msys2x64 shell it failed with old

---- test_cross_validate_abi_with_c stdout ----
        thread 'test_cross_validate_abi_with_c' panicked at 'called `Result::unwrap()` on an `Err` value: ParseIntError { kind: InvalidDigit }', libcore\result.rs:945:5

Glib failed with same error here too, while it works fine under cmd.exe

EPashkin · 2018-03-09T15:13:07Z

@tmiasko Rechecked again, previous problem self fixed.
Others as you says in #558 (comment) with exception that glib now be ok after gtk-rs/sys#81 😉
Gio still problem on Windows, but we can test it on Linux only or/and make fix later.
Gtk has problem in linux too, but again we can fix it separately.
And files generated don't have any changes on current crate as test is external and need special launch.
So I vote for merging this PR (it already was help us find 3 problem in glib).

@GuillaumeGomez, @sdroege You see any problem.

EPashkin · 2018-03-09T18:34:16Z

src/codegen/sys/tests.rs

+
+}
+
+


Please, remove empty lines

EPashkin · 2018-03-09T18:39:40Z

I think I came up with a solution to the problem with uncheckable types:
gir also generate second file abi_filter.rs (only if it don't exists) with:

pub fn filter(_name: &str) -> bool {
    true
}

in abi.rs it import this function and use it to skipping unwanted C check.
abi_filter.rs can be edited manually, for example for skipping types, not available on Windows.

EPashkin · 2018-03-09T18:41:42Z

Also IMHO abi.c and abi.rs need standard generated comment on beginning

ghost · 2018-03-09T18:59:27Z

I tried to address the uncheckable types using somewhat different approach.
In particular, I generate cfg conditions in the same way we do in user crates.
Of course, you also need to put those in the config to use this feature.

Unfortunately, this is only a partial solution. The main problem is that C
headers inclusion should be conditional as well. Doing that would be even more
involved.

EDIT:

Though, we could remove all non-portable headers from
gir first, and then include manually written header
that includes others as necessary based on ifdef checks.

EPashkin · 2018-03-09T19:17:31Z

@tmiasko Yes, we already have cfg_condition option for this, sadly it don't used in sys mode,
but can be used in get_rust_abi.
Agree about adding manual header file.

ghost · 2018-03-09T20:40:54Z

Changes since last version:

Rebased.
Parse CC, CFLAGS and CPPFLAGS using shell parsing rules for compatibility with GNU Make and other build tools.
Generate "DO NOT EDIT" headers.
Use cfg_condition in ABI tests.
Show ABI tests results summary on failure.
Include header for manual customization of ABI tests.

EPashkin · 2018-03-10T05:05:14Z

src/codegen/sys/tests.rs

+
+fn generate_abi_c(env: &Env, path: &Path, w: &mut Write) -> io::Result<()> {
+    info!("Generating file {:?}", path);
+    general::start_comments(w, &env.config)?;


Please add try!(writeln!(w, "")); after this, and for abi.rs too.

EPashkin · 2018-03-10T05:17:45Z

IMHO you can ignore secret-sys: it was added on initial stage and we decide don't publish it

EPashkin · 2018-03-10T05:32:42Z

Maybe we need special cfg_condition function for tests as current version always add feature="dox" that unneeded and may cause problem for tests.

EPashkin · 2018-03-10T05:42:40Z

src/codegen/sys/tests.rs

+                    None => return None,
+                    Some(name) => name,
+                };
+                if is_name_made_up(&name) {


EPashkin · 2018-03-10T05:51:45Z

src/codegen/sys/tests.rs

+    }
+
+    if results.failed + results.failed_compile != 0 {
+        panic!("FAILED\nABI test results: {} passed; {} failed (compilation errors: {})",


EPashkin · 2018-03-10T05:54:25Z

Maybe better don't include "includes" from .gir file to abi.c at all, and only add it to manual.h?

ghost · 2018-03-10T06:54:21Z

Regarding secret-sys, I wanted to test test generation :-) and selected
libsecret as a test subject, because it is small. Though, version on Ubuntu
Trusty, which is used on Travis CI, is not compatible with our GIR. I will try
using atk-sys now.

Regarding headers, they don't change too often, so putting them into separate
file but never overwriting them again (thus allowing for manual customization),
could work quite nicely. I will write that version later.

EDIT:

It looks like there have been ABI breaking changes in atk as well.
There are two compilation failures, and two ABI mismatches:

ABI mismatch for AtkDocumentIface
Rust: ABI { size: 80, alignment: 8 }
C:    ABI { size: 64, alignment: 8 }

ABI mismatch for AtkValueIface
Rust: ABI { size: 96, alignment: 8 }
C:    ABI { size: 56, alignment: 8 }

ghost · 2018-03-10T09:49:38Z

Added commit that put all headers into manual.h, which won't be regenerated unless removed.
Regarding feature="dox" in generated condition, I would leave it for now. It will compile fine, and you really shouldn't be running tests with dox.

EPashkin · 2018-03-10T10:44:54Z

src/codegen/sys/tests.rs

+    writeln!(w, "")?;
+
+    let ns = env.library.namespace(MAIN_NAMESPACE);
+    for include in ns.c_includes.iter() {


Please, change this to &ns.c_includes

Missed due to all those warnings. Is there some option in clippy to show lints only for files that have some changes in version control?

As I know currently this impossible

ghost · 2018-03-10T11:18:38Z

Fixed clippy warnings and rebased on top of #564.

Default build rules in GNU Make are constructed based on those environment variables. The rule is eventually evaluated through a shell. To retain compatibility with this behaviour build tools generally use shell parsing rules as well. And so do we!

When combinined with proper configuration changes this makes it possible to skip testing types which are known not to be available.

…cution fails.

Limit ABI tests to atk-sys, which is relatively small, to avoid excessive impact on CI runtime. ABI tests are disabled on Travis, because there have been ABI breaking changes since Ubuntu Trusty.

Headers generally need some manual tweaking. Some should be removed and others added, but generally don't change too often. Generate a separate file with headers, but don't overwrite it again unless it was removed.

EPashkin · 2018-03-11T08:22:24Z

@tmiasko Thanks

Cross validate Rust ABI with C ABI.

EPashkin mentioned this pull request Mar 8, 2018

GPid and GPollFD ABI mismatch for Windows gtk-rs/sys#82

Closed

EPashkin changed the title ~~[WIP] Cross validate Rust ABI with C ABI.~~ Cross validate Rust ABI with C ABI. Mar 9, 2018

EPashkin reviewed Mar 9, 2018

View reviewed changes

src/codegen/sys/tests.rs Outdated

}

Copy link

Member

EPashkin Mar 9, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please, remove empty lines

EPashkin reviewed Mar 10, 2018

View reviewed changes

tmiasko added 10 commits March 10, 2018 13:41

Cross validate Rust ABI with C ABI.

324f6f3

Generate "DO NOT EDIT" header for ABI tests.

05eae56

Use cfg attributes when generating ABI tests.

9af42e2

When combinined with proper configuration changes this makes it possible to skip testing types which are known not to be available.

Show ABI tests results summary on failure.

48bc963

Include header for manual customization of ABI tests.

d6338e1

Include stdout and stderr in addition to exit status when command exe…

6baf568

…cution fails.

Time for C99 is not yet ripe.

69d932d

Exercise ABI testing when running under Appveyor CI.

3abb3a8

Limit ABI tests to atk-sys, which is relatively small, to avoid excessive impact on CI runtime. ABI tests are disabled on Travis, because there have been ABI breaking changes since Ubuntu Trusty.

Put all headers from gir xml into manual.h

65e6691

Headers generally need some manual tweaking. Some should be removed and others added, but generally don't change too often. Generate a separate file with headers, but don't overwrite it again unless it was removed.

EPashkin merged commit 235b741 into gtk-rs:master Mar 11, 2018

This was referenced Mar 11, 2018

Cross validate the ABI of sys crates. #555

Closed

Add missing includes in GLib, GObject, GdkPixbuf, Gtk and Pango. gtk-rs/gir-files#16

Closed

vhdirk pushed a commit to vhdirk/gir that referenced this pull request Jul 6, 2018

Merge pull request gtk-rs#558 from tmiasko/check-abi

377a11c

Cross validate Rust ABI with C ABI.

Cross validate Rust ABI with C ABI. #558

Cross validate Rust ABI with C ABI. #558

Conversation

ghost commented Mar 1, 2018

EPashkin commented Mar 1, 2018

ghost commented Mar 1, 2018

EPashkin commented Mar 1, 2018

EPashkin commented Mar 1, 2018

ghost commented Mar 1, 2018

EPashkin commented Mar 1, 2018

EPashkin commented Mar 1, 2018

ghost commented Mar 2, 2018

EPashkin commented Mar 2, 2018

ghost commented Mar 2, 2018

EPashkin commented Mar 2, 2018

ghost commented Mar 3, 2018

EPashkin commented Mar 3, 2018

ghost commented Mar 3, 2018

EPashkin commented Mar 3, 2018

EPashkin commented Mar 9, 2018

Choose a reason for hiding this comment

EPashkin commented Mar 9, 2018 • edited Loading

EPashkin commented Mar 9, 2018

ghost commented Mar 9, 2018 • edited by ghost Loading

EPashkin commented Mar 9, 2018

ghost commented Mar 9, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

EPashkin commented Mar 10, 2018

EPashkin commented Mar 10, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

EPashkin commented Mar 10, 2018

ghost commented Mar 10, 2018 • edited by ghost Loading

ghost commented Mar 10, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ghost commented Mar 10, 2018

EPashkin commented Mar 11, 2018

EPashkin commented Mar 9, 2018 •

edited

Loading

ghost commented Mar 9, 2018 •

edited by ghost

Loading

ghost commented Mar 10, 2018 •

edited by ghost

Loading