ghpr-asia / wsdf

Wireshark Dissector Framework
Apache License 2.0
27 stars 12 forks source link

Failed to build epan-sys with bindgen: Unable to get layout information #18

Open charleywright opened 6 months ago

charleywright commented 6 months ago

I ran into this error while adding epan-sys to one of my projects and wanted to dig in and fix it. To reproduce simply try to compile the epan-sys crate with the bindgen feature enabled:

git clone https://github.com/ghpr-asia/wsdf --depth 1
cd wsdf/epan-sys
cargo build --features bindgen

Leading to:

--- stderr
thread 'main' panicked at ... bindgen-0.66.1/codegen/mod.rs:2139:33:
Unable to get layout information?

This is not the most helpful error message but can be improved by adding a warning before the panic:

diff --git a/bindgen/codegen/mod.rs b/bindgen/codegen/mod.rs
index 0b23e0a..816e93a 100644
--- a/bindgen/codegen/mod.rs
+++ b/bindgen/codegen/mod.rs
@@ -2216,7 +2216,11 @@ impl CodeGenerator for CompInfo {
         } else if is_union && !forward_decl {
             // TODO(emilio): It'd be nice to unify this with the struct path
             // above somehow.
-            let layout = layout.expect("Unable to get layout information?");
+            let Some(layout) = layout else {
+                println!("cargo::warning=We are about to crash, unable to get layout information for item\n{item:#?}");
+                panic!("Unable to get layout information?");
+            };
+            // let layout = layout.expect("Unable to get layout information?");
             if struct_layout.requires_explicit_align(layout) {
                 explicit_align = Some(layout.align);
             }

This leads to the wtap_pseudo_header union which is used in epan/packet_info.h (included by epan/packet.h) and defined in wiretap/wtap.h which isn't included. Since bindgen doesn't know about the definition there is no layout, leading to the panic.

There are two ways to fix this:

  1. Include wiretap/wtap.h in wrapper.h - See bindgen-fix1
  2. Define wtap_pseudo_header as an opaque type - See bindgen-fix2

The CONTRIBUTING document states that "If we need more types, functions, etc. from Wireshark, we can just add the respective .h files [to wrapper.h]" which would line up with fix 1. The only potential issue with this fix is the additional bloat in bindings.rs which is why I gave both options.

Thanks for writing and maintaining this crate, I look forward to implementing my dissector in Rust :)

Gbps commented 1 month ago

Changing wrapper.h to:

#include "wiretap/wtap.h"
#include "epan/packet.h"
#include "epan/decode_as.h"

Did the trick. Thanks!!

amitrahman1026 commented 2 weeks ago

@charleywright Do you have any more information about the system you were using at the time? I was working on this, and I realised that there were some issues regarding system dependencies for wireshark.

Noticed some cmake compilation errors when trying to build from source which I realise ended up being due to the tag of wireshark being used not up to date with certain fixes as described https://github.com/amitrahman1026/wsdf/pull/1.

I was trying to reproduce your layout issue but I am developing on an m1 mac. I did not encounter further build problems when trying to reproduce your error when running:

cargo build --features bindgen

Before this, I had however, set up relevant wireshark dependancies from here.