View Issue Details

IDProjectCategoryView StatusLast Update
0000483LDMud 3.3Runtimepublic2018-01-29 22:57
ReporterGnomi Assigned Tolars 
PrioritynormalSeveritycrashReproducibilityalways
Status resolvedResolutionfixed 
Platformi686OSDebian GNU/LinuxOS Version3.1
Product Version3.3.713 
Fixed in Version3.3.716 
Summary0000483: replace_program confuses variable sharing
DescriptionHi,

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.
TagsNo tags attached.

Activities

2006-09-11 15:25

 

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);
 
sharedvars.diff (3,512 bytes)   

Gnomi

2006-09-11 15:27

manager   ~0000516

I uploaded a diff which fixes this. This fix doesn't change the behaviour (as described above), because clone_object already searches for a blueprint and uses this instead of the given object.

lars

2007-10-06 23:28

reporter   ~0000556

Implemented - thanks!

Issue History

Date Modified Username Field Change
2006-08-10 07:44 Gnomi New Issue
2006-09-11 15:25 Gnomi File Added: sharedvars.diff
2006-09-11 15:27 Gnomi Note Added: 0000516
2007-10-06 23:28 lars Status new => resolved
2007-10-06 23:28 lars Fixed in Version => 3.3.716
2007-10-06 23:28 lars Resolution open => fixed
2007-10-06 23:28 lars Assigned To => lars
2007-10-06 23:28 lars Note Added: 0000556
2010-11-16 10:42 lars Source_changeset_attached => ldmud.git master 9e3fef8b
2018-01-29 19:59 lars Source_changeset_attached => ldmud.git master 9e3fef8b
2018-01-29 22:57 lars Source_changeset_attached => ldmud.git master 9e3fef8b