On 5/7/20 10:45 PM, Alexandre Julliard wrote:
Rémi Bernon rbernon@codeweavers.com writes:
+#if defined(__GNUC__) && ((__GNUC__ > 4) || ((__GNUC__ == 4) && (__GNUC_MINOR__ >= 7)))
- __atomic_store_n(&ptr->SystemTime.High2Time, system_time_high, __ATOMIC_SEQ_CST);
- __atomic_store_n(&ptr->SystemTime.LowPart, system_time_low, __ATOMIC_SEQ_CST);
- __atomic_store_n(&ptr->SystemTime.High1Time, system_time_high, __ATOMIC_SEQ_CST);
- __atomic_store_n(&ptr->InterruptTime.High2Time, interrupt_time_high, __ATOMIC_SEQ_CST);
- __atomic_store_n(&ptr->InterruptTime.LowPart, interrupt_time_low, __ATOMIC_SEQ_CST);
- __atomic_store_n(&ptr->InterruptTime.High1Time, interrupt_time_high, __ATOMIC_SEQ_CST);
- __atomic_store_n(&ptr->TickCount.High2Time, tick_count_high, __ATOMIC_SEQ_CST);
- __atomic_store_n(&ptr->TickCount.LowPart, tick_count_low, __ATOMIC_SEQ_CST);
- __atomic_store_n(&ptr->TickCount.High1Time, tick_count_high, __ATOMIC_SEQ_CST);
- __atomic_store_n(&ptr->TickCountLowDeprecated, tick_count_low, __ATOMIC_SEQ_CST);
Is this gcc-specific code really necessary?
I'm not sure it is strictly necessary but then it depends what guarantees we want to have. The volatile qualifier only acts as a compiler barrier and will make sure that the stores aren't optimized or reordered by the compiler. However it doesn't enforce anything on the CPU. I believe that X86 has global store ordering guarantees, but a quick search tells me that ARM or POWER don't, so the stores to the various members may be seen out of order from another CPU depending on the architecture.
Then, if it is about the __atomic builtins usage rather than __sync builtins that are sometimes used elsewhere, it's because these builtins provide cleaner semantics, as well as read-only and write-only operations -- whereas __sync only offers read+write operations. The page is mapped write-only in the server, so there was no guarantee that it was possible to read it. It's maybe a bit overkill and PROT_WRITE also somtimes implies PROT_READ, so maybe it should go this way and use __sync builtins instead?
+obj_handle_t get_usd_handle( const void *usd_init, data_size_t usd_size ) +{
- /* keep it the same as user_shared_data_size in ntdll */
- size_t size = 0x10000;
- struct fd *fd;
- if (sizeof(*kusd) != usd_size)
- {
set_error( STATUS_INVALID_PARAMETER );return 0;- }
- if (!kusd_mapping)
- {
if (!(kusd_mapping = create_mapping( NULL, NULL, 0, size, SEC_COMMIT, 0, 0, NULL )))return 0;make_object_static( kusd_mapping );if (!(fd = mapping_get_fd( kusd_mapping )))return 0;if ((kusd = mmap( NULL, size, PROT_WRITE, MAP_SHARED, get_unix_fd( fd ), 0 )) != MAP_FAILED){memcpy( kusd, usd_init, usd_size );kusd_set_current_time( NULL );}release_object( fd );- }
- if (kusd == MAP_FAILED)
- {
set_error( STATUS_NO_MEMORY );return 0;- }
- return alloc_handle( current->process, kusd_mapping, SECTION_QUERY|SECTION_MAP_READ, 0 );
+}
I still think it would be cleaner to use standard APIs for this, something like NtCreateSection("__wine_shared_user_data"), on STATUS_SUCCESS initialize it and tell the server about it, and on STATUS_OBJECT_NAME_EXISTS simply map it.
Alright.