wahn / rs_pbrt

Rust crate to implement a counterpart to the PBRT book's (3rd edition) C++ code. See also https://www.rs-pbrt.org/about ...
https://www.rs-pbrt.org
Other
811 stars 59 forks source link

Panic while trying to render due to UB in `linked-hash-map` #132

Closed expenses closed 3 years ago

expenses commented 3 years ago

I'm getting the following error while trying to run cargo run --release -- pbrt_ganesha/ganesha.pbrt:

thread 'main' panicked at 'attempted to leave type `linked_hash_map::Node<std::string::String, ply_rs::ply::ElementDef>` uninitialized, which is invalid', /rustc/2fd73fabe469357a12c2c974c140f67e7cdd76d0/library/core/src/mem/mod.rs:671:9
stack backtrace:
   0: rust_begin_unwind
             at /rustc/2fd73fabe469357a12c2c974c140f67e7cdd76d0/library/std/src/panicking.rs:493:5
   1: core::panicking::panic_fmt
             at /rustc/2fd73fabe469357a12c2c974c140f67e7cdd76d0/library/core/src/panicking.rs:92:14
   2: core::panicking::panic
             at /rustc/2fd73fabe469357a12c2c974c140f67e7cdd76d0/library/core/src/panicking.rs:50:5
   3: <linked_hash_map::LinkedHashMap<alloc::string::String,V> as ply_rs::ply::key_map::Addable<V>>::add
   4: rs_pbrt::shapes::plymesh::create_ply_mesh
   5: rs_pbrt::core::api::get_shapes_and_materials
   6: rs_pbrt::core::api::pbrt_shape
   7: rs_pbrt::parse_line
   8: rs_pbrt::parse_file
   9: rs_pbrt::main

This problem is caused by Undefined Behaviour in versions 0.5.3 and below of linked-hash-map: https://github.com/contain-rs/linked-hash-map/pull/106 which is imported via ply-rs.

I have a PR up for ply-rs that fixes this issue: https://github.com/Fluci/ply-rs/pull/18 so for now setting ply-rs in Cargo.toml to ply-rs = { git = "https://github.com/expenses/ply-rs", branch = "linked-hash-map-bump" } works.

It seems like ply-rs hasn't been updated for a while so it's up to you whether you want to wait for it to update or to fix this in a different way.

wahn commented 3 years ago

Hi @expenses , Thanks for reporting this. I just confirmed your reported problem and the fix you suggest seems to work. Maybe I wait a bit if the PR gets accepted and a new release of ply-rs will be created. Thanks, again

wahn commented 3 years ago

Hi @expenses ,

could you try with the latest commits, please? For me the ganesha.pbrt scene seems to work now. Because I do not ship example scenes with the Rust crate anymore you might have to clone this repository:

https://gitlab.com/jdb-walter/rs-pbrt-test-scenes

After you cloned the repository you have to unpack the scene and make sure that you are rendering from within that directory:

$ cd pbrt/
$ tar zxfv pbrt_ganesha.tar.gz
$ cd pbrt_ganesha/
$ rs_pbrt ganesha.pbrt
pbrt version 0.9.1 (v0.9.1-1-g10bd3b9) [Detected 28 cores]
Copyright (c) 2016-2021 Jan Douglas Bert Walter.
Rust code based on C++ code by Matt Pharr, Greg Humphreys, and Wenzel Jakob.
Integrator "path"
  "integer maxdepth" [4]
Sampler "halton"
  "integer pixelsamples" [512]
Film "image"
  "string filename" ["ganesha.exr"]
  "integer xresolution" [720]
  "integer yresolution" [720]
Rendering with 28 thread(s) ...
2116 / 2116 [========================================================================] 100.00 % 32.03/s 
Writing image "pbrt.png" with bounds Bounds2i { p_min: Point2i { x: 0, y: 0 }, p_max: Point2i { x: 720, y: 720 } }

It would be great if you could run that test, so I could close the issue ...

Thanks