[Bf-blender-cvs] [0e3d1eee15b] master: Fix (unreported) crash when undoing after ID deletion.
Bastien Montagne
noreply at git.blender.org
Fri Feb 8 18:58:21 CET 2019
Commit: 0e3d1eee15b7b6dadaddaa7f1f4b8d30b80e6792
Author: Bastien Montagne
Date: Fri Feb 8 18:45:57 2019 +0100
Branches: master
https://developer.blender.org/rB0e3d1eee15b7b6dadaddaa7f1f4b8d30b80e6792
Fix (unreported) crash when undoing after ID deletion.
Yes, we do can undo an ID deletion now.
However, this requires extra care in UI 'remapping' to new IDs step
(when undoing, we do not fully reload the UI from saved .blend).
Otherwise, new UI (i.e. one from saved .blend file) might reference
IDs that where freed in old bmain (the one before the undo), we cannot
use those to get ID name then, that would be a nasty use-after-free!
To prevent this, we generate a GSet of all valid ID pointers at that
time (i.e. those found in both old and new Main's), and ensure any ID
we try to remap by its name is in that GSet. Otherwise, there is no
possible remapping, just return NULL.
===================================================================
M source/blender/blenkernel/BKE_library_idmap.h
M source/blender/blenkernel/intern/blendfile.c
M source/blender/blenkernel/intern/library_idmap.c
M source/blender/blenloader/BLO_readfile.h
M source/blender/blenloader/intern/readfile.c
===================================================================
diff --git a/source/blender/blenkernel/BKE_library_idmap.h b/source/blender/blenkernel/BKE_library_idmap.h
index ec814ac73de..0e16aa15984 100644
--- a/source/blender/blenkernel/BKE_library_idmap.h
+++ b/source/blender/blenkernel/BKE_library_idmap.h
@@ -26,8 +26,10 @@ struct IDNameLib_Map;
struct Main;
struct IDNameLib_Map *BKE_main_idmap_create(
- struct Main *bmain)
- ATTR_WARN_UNUSED_RESULT ATTR_NONNULL();
+ struct Main *bmain,
+ const bool create_valid_ids_set,
+ struct Main *old_bmain)
+ ATTR_WARN_UNUSED_RESULT ATTR_NONNULL(1);
void BKE_main_idmap_destroy(
struct IDNameLib_Map *id_typemap)
ATTR_NONNULL();
diff --git a/source/blender/blenkernel/intern/blendfile.c b/source/blender/blenkernel/intern/blendfile.c
index 34cc0a69c2e..51c5b52d088 100644
--- a/source/blender/blenkernel/intern/blendfile.c
+++ b/source/blender/blenkernel/intern/blendfile.c
@@ -203,7 +203,7 @@ static void setup_app_data(
}
/* BKE_blender_globals_clear will free G_MAIN, here we can still restore pointers */
- blo_lib_link_restore(bfd->main, CTX_wm_manager(C), curscene, cur_view_layer);
+ blo_lib_link_restore(bmain, bfd->main, CTX_wm_manager(C), curscene, cur_view_layer);
if (win) {
curscene = win->scene;
}
diff --git a/source/blender/blenkernel/intern/library_idmap.c b/source/blender/blenkernel/intern/library_idmap.c
index 2745a1fe275..3ab5cbaeef3 100644
--- a/source/blender/blenkernel/intern/library_idmap.c
+++ b/source/blender/blenkernel/intern/library_idmap.c
@@ -65,6 +65,7 @@ struct IDNameLib_TypeMap {
struct IDNameLib_Map {
struct IDNameLib_TypeMap type_maps[MAX_LIBARRAY];
struct Main *bmain;
+ struct GSet *valid_id_pointers;
};
static struct IDNameLib_TypeMap *main_idmap_from_idcode(struct IDNameLib_Map *id_map, short id_type)
@@ -77,7 +78,20 @@ static struct IDNameLib_TypeMap *main_idmap_from_idcode(struct IDNameLib_Map *id
return NULL;
}
-struct IDNameLib_Map *BKE_main_idmap_create(struct Main *bmain)
+/**
+ * Generate mapping from ID type/name to ID pointer for given \a bmain.
+ *
+ * \note When used during undo/redo, there is no guaranty that ID pointers from UI area
+ * are not pointing to freed memory (when some IDs have been deleted). To avoid crashes
+ * in those cases, one can provide the 'old' (aka current) Main databse as reference.
+ * #BKE_main_idmap_lookup_id will then check that given ID does exist in \a old_bmain
+ * before trying to use it.
+ *
+ * \param create_valid_ids_set If \a true, generate a reference to prevent freed memory accesses.
+ * \param old_bmain If not NULL, its IDs will be added the the valid references set.
+ */
+struct IDNameLib_Map *BKE_main_idmap_create(
+ struct Main *bmain, const bool create_valid_ids_set, struct Main *old_bmain)
{
struct IDNameLib_Map *id_map = MEM_mallocN(sizeof(*id_map), __func__);
@@ -92,6 +106,16 @@ struct IDNameLib_Map *BKE_main_idmap_create(struct Main *bmain)
id_map->bmain = bmain;
+ if (create_valid_ids_set) {
+ id_map->valid_id_pointers = BKE_main_gset_create(bmain, NULL);
+ if (old_bmain != NULL) {
+ id_map->valid_id_pointers = BKE_main_gset_create(old_bmain, id_map->valid_id_pointers);
+ }
+ }
+ else {
+ id_map->valid_id_pointers = NULL;
+ }
+
return id_map;
}
@@ -151,7 +175,15 @@ ID *BKE_main_idmap_lookup(struct IDNameLib_Map *id_map, short id_type, const cha
ID *BKE_main_idmap_lookup_id(struct IDNameLib_Map *id_map, const ID *id)
{
- return BKE_main_idmap_lookup(id_map, GS(id->name), id->name + 2, id->lib);
+ /* When used during undo/redo, this function cannot assume that given id points to valid memory
+ * (i.e. has not been freed), so it has to check that it does exist in 'old' (aka current) Main database.
+ * Otherwise, we cannot provide new ID pointer that way (would crash accessing freed memory
+ * when trying to get ID name).
+ */
+ if (id_map->valid_id_pointers == NULL || BLI_gset_haskey(id_map->valid_id_pointers, id)) {
+ return BKE_main_idmap_lookup(id_map, GS(id->name), id->name + 2, id->lib);
+ }
+ return NULL;
}
void BKE_main_idmap_destroy(struct IDNameLib_Map *id_map)
@@ -165,6 +197,10 @@ void BKE_main_idmap_destroy(struct IDNameLib_Map *id_map)
}
}
+ if (id_map->valid_id_pointers != NULL) {
+ BLI_gset_free(id_map->valid_id_pointers, NULL);
+ }
+
MEM_freeN(id_map);
}
diff --git a/source/blender/blenloader/BLO_readfile.h b/source/blender/blenloader/BLO_readfile.h
index 90f46ccd49d..e8bfeecb305 100644
--- a/source/blender/blenloader/BLO_readfile.h
+++ b/source/blender/blenloader/BLO_readfile.h
@@ -144,7 +144,7 @@ void *BLO_library_read_struct(struct FileData *fd, struct BHead *bh, const char
/* internal function but we need to expose it */
void blo_lib_link_restore(
- struct Main *newmain, struct wmWindowManager *curwm,
+ struct Main *oldmain, struct Main *newmain, struct wmWindowManager *curwm,
struct Scene *curscene, struct ViewLayer *cur_render_layer);
typedef void (*BLOExpandDoitCallback) (void *fdhandle, struct Main *mainvar, void *idv);
diff --git a/source/blender/blenloader/intern/readfile.c b/source/blender/blenloader/intern/readfile.c
index fa3d3450f06..f511bf93be1 100644
--- a/source/blender/blenloader/intern/readfile.c
+++ b/source/blender/blenloader/intern/readfile.c
@@ -7635,9 +7635,9 @@ static void lib_link_workspace_layout_restore(struct IDNameLib_Map *id_map, Main
* Used to link a file (without UI) to the current UI.
* Note that it assumes the old pointers in UI are still valid, so old Main is not freed.
*/
-void blo_lib_link_restore(Main *newmain, wmWindowManager *curwm, Scene *curscene, ViewLayer *cur_view_layer)
+void blo_lib_link_restore(Main *oldmain, Main *newmain, wmWindowManager *curwm, Scene *curscene, ViewLayer *cur_view_layer)
{
- struct IDNameLib_Map *id_map = BKE_main_idmap_create(newmain);
+ struct IDNameLib_Map *id_map = BKE_main_idmap_create(newmain, true, oldmain);
for (WorkSpace *workspace = newmain->workspaces.first; workspace; workspace = workspace->id.next) {
ListBase *layouts = BKE_workspace_layouts_get(workspace);
More information about the Bf-blender-cvs
mailing list