On Mon, Apr 28, 2014 at 12:25 PM, Thomas Schmitt <scdbac...@gmx.net> wrote:
> Hi, > > Patrick Baggett: > > Could you explain the context around this code? Perhaps the source is > > not really "alignment safe" and could use some patching upstream? I'd > > be happy to provide advice or code samples. > > The context was misposted to bug report 731806 as message #87: > https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=731806#87 > (but not with Cc to debian-sparc list). > > Upstream is myself. :)) > The code in question is by my unreachable libburn predecessors, though. > > It belongs to the preparation of a thread start for one of five > occasions in libburn. SIGBUS happens at > http://libburnia-project.org/browser/libburn/trunk/libburn/async.c#L149 > > The caller has a local struct (i.e. on stack) like (#L592, #L692): > > struct write_opts write; > ... > add_worker(Burnworker_type_writE, d, > (WorkerFunc) write_disc_worker_func, &o); > > The called function gets its address as parameter "data" (#L135): > > static void add_worker(int w_type, struct burn_drive *d, > WorkerFunc f, void *data) > > has a struct on heap (#L102, #L138, #L146): > > struct w_list{ > ... > union w_list_data > { > ... > struct write_opts write; > ... > } u; > } > ... > struct w_list *a; > ... > a = calloc(1, sizeof(struct w_list)); > > The gesture which causes the SIGBUS is (#L149) > > a->u = *(union w_list_data *)data; > > which is not what i personally would use, but should be fully legal > nevertheless. > > I'm not sure what the problem is exactly here. 64-bit SPARC will use ldx to load 64-bit quantities, which can cause problems if the source data is only 4-byte aligned, but that doesn't seem to be the case here. I really need a disassembly and to be able to probe the runtime stack a bit, so that really means that I need to build the code. :) I think as a more meta-problem is this: the code's logic for "how much" to copy is wrong. It copies too many bytes on many cases and violates the C contract that you'll only copy like objects using "=". Imagine: sizeof(struct erase_ops) == 8 (32-bit ABI) sizeof(w_list_data) == at least 12 (32-bit ABI). Suppose: data = &some_erase_ops a->u = *(union w_list_data *)data; // copies 12 bytes from 8 byte area, can cause crashes if the last 4 bytes are into an unmapped page. The correct way to do this would be to have add_worker() pass a const w_list_data* with the appropriate field(s) in '.u' filled out. Otherwise, you're copying unlike objects *of different sizes* and that's never safe. --- Patrick > The SIGBUS vanishes if i compile without gcc -O2, or if i replace > the "a->u =" gesture by > > memcpy(&(a->u), data, sizeof(union w_list_data)); > > which i deem equivalent (and more my personal style). > > > Have a nice day :) > > Thomas > >
