View Issue Details
ID | Project | Category | View Status | Date Submitted | Last Update |
---|---|---|---|---|---|
0000483 | LDMud 3.3 | Runtime | public | 2006-08-10 05:44 | 2018-01-29 21:57 |
Reporter | Gnomi | Assigned To | |||
Priority | normal | Severity | crash | Reproducibility | always |
Status | resolved | Resolution | fixed | ||
Platform | i686 | OS | Debian GNU/Linux | OS Version | 3.1 |
Product Version | 3.3.713 | ||||
Fixed in Version | 3.3.716 | ||||
Summary | 0000483: replace_program confuses variable sharing | ||||
Description | Hi, while playing around with the pragma share_variables and replace_program, I tried to provoke a crash and succeeded: 2006.08.10 00:48:10 (free_svalue) Illegal svalue 0xc01f9d4 type 285 ... apps/closure_container <lambda ?> line 0 b9cb4cf: 98 restore_arg_frame (2: 1) line 0 b9cb4d0: 24 return (1: 0) 8555716: 24 25 72 166 50 8 4 0 No program to trace. 2006.08.10 00:48:10 LDMud aborting on fatal error. #0 0x080da4b0 in fatal (fmt=0x8141348 "\002") at simulate.c:586 586 *((char*)0) = 0/a; (gdb) bt #0 0x080da4b0 in fatal (fmt=0x8141348 "\002") at simulate.c:586 0000001 0x0807f2cf in int_free_svalue (v=0xc01f9d4) at interpret.c:1097 0000002 0x080dd92e in remove_object (ob=0xbf01748) at simulate.c:2857 0000003 0x080dda77 in handle_newly_destructed_objects () at simulate.c:2947 0000004 0x08052eea in backend () at backend.c:476 0000005 0x080a7709 in main (argc=0, argv=0x0) at main.c:615 I used the following files (and initialization by __INIT): --- i/x.c: --- int dubdidu = 20; void dummy() {} --- i/y.c: --- #pragma share_variables inherit "x"; int a = 10; int b = 10; int c = 10; int d = 10; int e = 10; int f = 10; int g = 10; int h = 10; int i = 10; int j = 10; int k = 10; int l = 10; int m = 10; int n = 10; int o = 10; int p = 10; int q = 10; int r = 10; int s = 10; int t = 10; int u = 10; int v = 10; int w = 10; int x = 10; int y = 10; int z = 10; void rpy() { replace_program(); } --- obj/z.c: --- inherit "../i/y"; void rpz() { replace_program(); } --- app.c: --- void step1() { object y = load_object(__PATH__(0) "i/y"); object z = load_object(__PATH__(0) "obj/z"); y->rpy(); z->rpz(); } void step2() { destruct(clone_object(__PATH__(0) "obj/z")); } ------------- Calling app->step1() results in three objects: i/x with the program of i/x.c and one __INITialized variable. i/y with the program of i/x.c and therefore also one __INITialized variable. obj/z with the program of i/y.c, one __INITialized variable and 26 shared variables. In this constellation the blueprint (given by the efun blueprint) of obj/z is i/y, but the program of i/y is i/x.c, so the blueprints program does not correspond to the program of its clones. When cloning obj/z by app->step2(), init_object_variables looks for the blueprint and copies shared variables from the blueprint. But as the blueprint (i/y with the program i/x.c) does only have one variable, the needed 26 shared values are random bytes from memory beyond the variable block. Freeing them may then result in a crash. I would suggest two things to solve this. First replace_program, when called from a blueprint, should set the blueprint pointer of the replaced program to NULL, because then the blueprint won't be a blueprint for this program anymore. Second init_object_variables should copy the variables from the template given to clone_object and not necessarily from prog->blueprint. The second proposal may break things (especially when cloning from a clone, there it is now expected, that the variables are taken from the blueprint), but I think it's more intuitive. Changing this behaviour only in the case, when the template is not a clone, seems a half-hearted solution to me. And this change will allow applications as Fiona described in her Mail <Pine.LNX.4.33.0509231010370.16774-100000@hasyl03.desy.de> from September, 23rd 2005 on the LDMud-Talk list (for a specific weapon inherit a generic weapon, set some global variables like the weapon type and strength, do a replace_program and let others clone then this specific weapon). It would also avoid the "Can't initialize object 'obj/xyz#123': no blueprint." errors, when the inherit (i/y in this case) was destroyed meanwhile (which can happen very easily). Greetings, Gnomi. | ||||
Tags | No tags attached. | ||||
Attached Files | sharedvars.diff (3,512 bytes)
Index: src/object.c =================================================================== --- src/object.c (Revision 2312) +++ src/object.c (Arbeitskopie) @@ -448,7 +448,7 @@ /*-------------------------------------------------------------------------*/ void -init_object_variables (object_t *ob) +init_object_variables (object_t *ob, object_t *templ) /* The variables of object <ob> are initialized. * First, if <ob> is a clone, all variables marked as !VAR_INITIALIZED are @@ -463,20 +463,14 @@ /* For clones, copy the shared variable values */ if ((ob->flags & O_CLONE)) { - object_t *bp; int i; variable_t *p_vars; svalue_t *ob_vars, *bp_vars; - bp = ob->prog->blueprint; - if (!bp || bp->flags & O_DESTRUCTED) - { - if (bp) free_object(bp, "init_object_variables"); - ob->prog->blueprint = NULL; + if (!templ || templ->flags & O_DESTRUCTED) bp_vars = NULL; - } else - bp_vars = bp->variables; + bp_vars = templ->variables; ob_vars = ob->variables; p_vars = ob->prog->variables; @@ -1065,6 +1059,16 @@ r_ob->ob->variables = new_vars; } /* if (change in vars) */ + /* If this is a blueprint object, NULL out the pointer in the program, + * because this isn't the blueprint for this program anymore. + */ + if (r_ob->ob->prog->blueprint == r_ob->ob) + { + r_ob->ob->prog->blueprint = NULL; + remove_prog_swap(r_ob->ob->prog, MY_TRUE); + free_object(r_ob->ob, "replace_programs: blueprint reference"); + } + /* Replace the old program with the new one */ old_prog = r_ob->ob->prog; r_ob->new_prog->ref++; Index: src/object.h =================================================================== --- src/object.h (Revision 2312) +++ src/object.h (Arbeitskopie) @@ -295,7 +295,7 @@ extern void dealloc_object(object_t *, const char * file, int line); #endif extern object_t *get_empty_object(int num_var); -extern void init_object_variables (object_t *ob); +extern void init_object_variables (object_t *ob, object_t *templ); extern svalue_t *v_function_exists(svalue_t *sp, int num_arg); extern svalue_t *f_functionlist(svalue_t *sp); Index: src/simulate.c =================================================================== --- src/simulate.c (Revision 2312) +++ src/simulate.c (Arbeitskopie) @@ -2117,7 +2117,7 @@ { /* The master object is loaded with no current object */ current_object = NULL; - init_object_variables(ob); + init_object_variables(ob, NULL); reset_object(ob, create_super ? H_CREATE_SUPER : H_CREATE_OB); /* If the master inherits anything -Ugh- we have to have @@ -2128,7 +2128,7 @@ else { current_object = save_current; - init_object_variables(ob); + init_object_variables(ob, NULL); reset_object(ob, create_super ? H_CREATE_SUPER : H_CREATE_OB); } } @@ -2337,7 +2337,7 @@ push_ref_object(inter_sp, ob, "clone_object"); push_ref_string(inter_sp, new_ob->name); give_uid_to_object(new_ob, H_CLONE_UIDS, 2); - init_object_variables(new_ob); + init_object_variables(new_ob, ob); reset_object(new_ob, H_CREATE_CLONE); command_giver = check_object(save_command_giver); | ||||
Date Modified | Username | Field | Change |
---|---|---|---|
2006-08-10 05:44 | Gnomi | New Issue | |
2006-09-11 13:25 | Gnomi | File Added: sharedvars.diff | |
2006-09-11 13:27 | Gnomi | Note Added: 0000516 | |
2007-10-06 21:28 |
|
Status | new => resolved |
2007-10-06 21:28 |
|
Fixed in Version | => 3.3.716 |
2007-10-06 21:28 |
|
Resolution | open => fixed |
2007-10-06 21:28 |
|
Assigned To | => lars |
2007-10-06 21:28 |
|
Note Added: 0000556 | |
2010-11-16 09:42 |
|
Source_changeset_attached | => ldmud.git master 9e3fef8b |
2018-01-29 18:59 |
|
Source_changeset_attached | => ldmud.git master 9e3fef8b |
2018-01-29 21:57 |
|
Source_changeset_attached | => ldmud.git master 9e3fef8b |